This series adds devicetree nodes to support interconnects and usb for qdu/qru1000.
This is based on previously sent driver series[1].
------
Changes in v3:
* As per comments on upstream[2], to get role-switch working on QDU/QRU1000, it was recommended to
use the actual TI switch driver. Since driver doesn't have the functionality to provide role-switch
based on gpio, thus reverting back USB dr_mode to peripheral and removed the remote end-point nodes
and usb-conn-gpio based role switch functionality.
* Link to v2: https://lore.kernel.org/linux-arm-msm/[email protected]/
Changes in v2:
* Changes qmpphy node name
* Changes dr_mode to otg and added USB-B port USB role switch
* Dropped maximum-speed property from usb dwc3 node
* Link to v1: https://lore.kernel.org/linux-arm-msm/[email protected]/
[1] https://lore.kernel.org/linux-arm-msm/[email protected]/
[2] https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
------
Komal Bajaj (3):
arm64: dts: qcom: qdu1000: Add USB3 and PHY support
arm64: dts: qcom: qdu1000-idp: enable USB nodes
arm64: dts: qcom: qru1000-idp: enable USB nodes
arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 23 +++++
arch/arm64/boot/dts/qcom/qdu1000.dtsi | 120 +++++++++++++++++++++++
arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++
3 files changed, 166 insertions(+)
--
2.42.0
Enable both USB controllers and associated hsphy and qmp phy
nodes on QRU1000 IDP.
Co-developed-by: Amrit Anand <[email protected]>
Signed-off-by: Amrit Anand <[email protected]>
Signed-off-by: Komal Bajaj <[email protected]>
---
arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qru1000-idp.dts b/arch/arm64/boot/dts/qcom/qru1000-idp.dts
index 2a862c83309e..1c781d9e24cf 100644
--- a/arch/arm64/boot/dts/qcom/qru1000-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qru1000-idp.dts
@@ -467,3 +467,26 @@ &tlmm {
&uart7 {
status = "okay";
};
+
+&usb_1 {
+ status = "okay";
+};
+
+&usb_1_dwc3 {
+ dr_mode = "peripheral";
+};
+
+&usb_1_hsphy {
+ vdda-pll-supply = <&vreg_l8a_0p91>;
+ vdda18-supply = <&vreg_l14a_1p8>;
+ vdda33-supply = <&vreg_l2a_2p3>;
+
+ status = "okay";
+};
+
+&usb_1_qmpphy {
+ vdda-phy-supply = <&vreg_l8a_0p91>;
+ vdda-pll-supply = <&vreg_l3a_1p2>;
+
+ status = "okay";
+};
--
2.42.0
On Thu, 2 May 2024 at 12:04, Komal Bajaj <[email protected]> wrote:
>
> This series adds devicetree nodes to support interconnects and usb for qdu/qru1000.
> This is based on previously sent driver series[1].
>
> ------
> Changes in v3:
> * As per comments on upstream[2], to get role-switch working on QDU/QRU1000, it was recommended to
> use the actual TI switch driver. Since driver doesn't have the functionality to provide role-switch
> based on gpio, thus reverting back USB dr_mode to peripheral and removed the remote end-point nodes
> and usb-conn-gpio based role switch functionality.
This is not correct. The recommendation was to describe hardware properly.
Which means adding schema description, adding ti,your-switch
compatible to the usb-conn-gpio.c driver, etc.
> * Link to v2: https://lore.kernel.org/linux-arm-msm/[email protected]/
>
> Changes in v2:
> * Changes qmpphy node name
> * Changes dr_mode to otg and added USB-B port USB role switch
> * Dropped maximum-speed property from usb dwc3 node
> * Link to v1: https://lore.kernel.org/linux-arm-msm/[email protected]/
>
> [1] https://lore.kernel.org/linux-arm-msm/[email protected]/
> [2] https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
> ------
>
> Komal Bajaj (3):
> arm64: dts: qcom: qdu1000: Add USB3 and PHY support
> arm64: dts: qcom: qdu1000-idp: enable USB nodes
> arm64: dts: qcom: qru1000-idp: enable USB nodes
>
> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 23 +++++
> arch/arm64/boot/dts/qcom/qdu1000.dtsi | 120 +++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++
> 3 files changed, 166 insertions(+)
>
> --
> 2.42.0
>
>
--
With best wishes
Dmitry
On 5/2/2024 2:39 PM, Dmitry Baryshkov wrote:
> On Thu, 2 May 2024 at 12:04, Komal Bajaj <[email protected]> wrote:
>>
>> This series adds devicetree nodes to support interconnects and usb for qdu/qru1000.
>> This is based on previously sent driver series[1].
>>
>> ------
>> Changes in v3:
>> * As per comments on upstream[2], to get role-switch working on QDU/QRU1000, it was recommended to
>> use the actual TI switch driver. Since driver doesn't have the functionality to provide role-switch
>> based on gpio, thus reverting back USB dr_mode to peripheral and removed the remote end-point nodes
>> and usb-conn-gpio based role switch functionality.
>
> This is not correct. The recommendation was to describe hardware properly.
> Which means adding schema description, adding ti,your-switch
> compatible to the usb-conn-gpio.c driver, etc.
>
Hi Dmitry,
Sorry for the confusion. In the comments [1],
"So the compatible string should be "ti,hd3ss3220". Which is fine to be
used in the platform driver. Just describe the differences in the
schema."
The compatible "ti,hd3ss3220" is already associated with a TI switch
driver [2]. But it works based on I2C. So we assumed you wanted us to
make changes to [2] by adding GPIO functionality (which usb-conn-gpio
exactly does), since the compatible you suggested matched with the TI
driver.
If it was to add compatible in usb-conn-gpio, then we can support OTG
functionality with no schema changes I believe, but the compatible
string might need a different name to avoid clashing with the name in [2].
[1]:
https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
[2]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/hd3ss3220.c?h=v6.9-rc2
Regards,
Krishna,
>> * Link to v2: https://lore.kernel.org/linux-arm-msm/[email protected]/
>>
>> Changes in v2:
>> * Changes qmpphy node name
>> * Changes dr_mode to otg and added USB-B port USB role switch
>> * Dropped maximum-speed property from usb dwc3 node
>> * Link to v1: https://lore.kernel.org/linux-arm-msm/[email protected]/
>>
>> [1] https://lore.kernel.org/linux-arm-msm/[email protected]/
>> [2] https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
>> ------
>>
>> Komal Bajaj (3):
>> arm64: dts: qcom: qdu1000: Add USB3 and PHY support
>> arm64: dts: qcom: qdu1000-idp: enable USB nodes
>> arm64: dts: qcom: qru1000-idp: enable USB nodes
>>
>> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 23 +++++
>> arch/arm64/boot/dts/qcom/qdu1000.dtsi | 120 +++++++++++++++++++++++
>> arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++
>> 3 files changed, 166 insertions(+)
>>
>> --
>> 2.42.0
>>
>>
>
>
On Thu, 2 May 2024 at 12:48, Krishna Kurapati PSSNV
<[email protected]> wrote:
>
>
>
> On 5/2/2024 2:39 PM, Dmitry Baryshkov wrote:
> > On Thu, 2 May 2024 at 12:04, Komal Bajaj <[email protected]> wrote:
> >>
> >> This series adds devicetree nodes to support interconnects and usb for qdu/qru1000.
> >> This is based on previously sent driver series[1].
> >>
> >> ------
> >> Changes in v3:
> >> * As per comments on upstream[2], to get role-switch working on QDU/QRU1000, it was recommended to
> >> use the actual TI switch driver. Since driver doesn't have the functionality to provide role-switch
> >> based on gpio, thus reverting back USB dr_mode to peripheral and removed the remote end-point nodes
> >> and usb-conn-gpio based role switch functionality.
> >
> > This is not correct. The recommendation was to describe hardware properly.
> > Which means adding schema description, adding ti,your-switch
> > compatible to the usb-conn-gpio.c driver, etc.
> >
>
> Hi Dmitry,
>
> Sorry for the confusion. In the comments [1],
>
> "So the compatible string should be "ti,hd3ss3220". Which is fine to be
> used in the platform driver. Just describe the differences in the
> schema."
>
> The compatible "ti,hd3ss3220" is already associated with a TI switch
> driver [2]. But it works based on I2C. So we assumed you wanted us to
> make changes to [2] by adding GPIO functionality (which usb-conn-gpio
> exactly does), since the compatible you suggested matched with the TI
> driver.
First of all, please don't make assumptions. It's better to ask rather
than making assumptions which turn up to be incorrect.
Compatibles describe hardware. DT describes hardware. There are no
drivers in question (yet).
You have TI switch on your board, so you have to use "ti,hd3ss3220" to
describe it.
Existing schema describes it as an I2C device. You have to extend the
schema to allow non-i2c attachment. Describe GPIOs, make reg optional.
Make this description purely from the datasheet and usb-c-connector
point of view.
> If it was to add compatible in usb-conn-gpio, then we can support OTG
> functionality with no schema changes I believe, but the compatible
> string might need a different name to avoid clashing with the name in [2].
And this is the second, largely independent question. The
usb-conn-gpio driver is a platform driver.The existing hd3ss3220.c
driver is an I2C one. There is no clash between them.
Note, unlike plain gpio-b-connector, the switch supports more pins and
actually provides USB-C information to the host even when used in the
dumb mode. Thus it might be better to add a separate driver that
registers typec port and reports USB-C events.
>
> [1]:
> https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
>
> [2]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/hd3ss3220.c?h=v6.9-rc2
>
> Regards,
> Krishna,
>
> >> * Link to v2: https://lore.kernel.org/linux-arm-msm/[email protected]/
> >>
> >> Changes in v2:
> >> * Changes qmpphy node name
> >> * Changes dr_mode to otg and added USB-B port USB role switch
> >> * Dropped maximum-speed property from usb dwc3 node
> >> * Link to v1: https://lore.kernel.org/linux-arm-msm/[email protected]/
> >>
> >> [1] https://lore.kernel.org/linux-arm-msm/[email protected]/
> >> [2] https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
> >> ------
> >>
> >> Komal Bajaj (3):
> >> arm64: dts: qcom: qdu1000: Add USB3 and PHY support
> >> arm64: dts: qcom: qdu1000-idp: enable USB nodes
> >> arm64: dts: qcom: qru1000-idp: enable USB nodes
> >>
> >> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 23 +++++
> >> arch/arm64/boot/dts/qcom/qdu1000.dtsi | 120 +++++++++++++++++++++++
> >> arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++
> >> 3 files changed, 166 insertions(+)
> >>
> >> --
> >> 2.42.0
> >>
> >>
> >
> >
--
With best wishes
Dmitry
On 5/2/2024 5:14 PM, Dmitry Baryshkov wrote:
> On Thu, 2 May 2024 at 12:48, Krishna Kurapati PSSNV
> <[email protected]> wrote:
>>
>>
>>
>> On 5/2/2024 2:39 PM, Dmitry Baryshkov wrote:
>>> On Thu, 2 May 2024 at 12:04, Komal Bajaj <[email protected]> wrote:
>>>>
>>>> This series adds devicetree nodes to support interconnects and usb for qdu/qru1000.
>>>> This is based on previously sent driver series[1].
>>>>
>>>> ------
>>>> Changes in v3:
>>>> * As per comments on upstream[2], to get role-switch working on QDU/QRU1000, it was recommended to
>>>> use the actual TI switch driver. Since driver doesn't have the functionality to provide role-switch
>>>> based on gpio, thus reverting back USB dr_mode to peripheral and removed the remote end-point nodes
>>>> and usb-conn-gpio based role switch functionality.
>>>
>>> This is not correct. The recommendation was to describe hardware properly.
>>> Which means adding schema description, adding ti,your-switch
>>> compatible to the usb-conn-gpio.c driver, etc.
>>>
>>
>> Hi Dmitry,
>>
>> Sorry for the confusion. In the comments [1],
>>
>> "So the compatible string should be "ti,hd3ss3220". Which is fine to be
>> used in the platform driver. Just describe the differences in the
>> schema."
>>
>> The compatible "ti,hd3ss3220" is already associated with a TI switch
>> driver [2]. But it works based on I2C. So we assumed you wanted us to
>> make changes to [2] by adding GPIO functionality (which usb-conn-gpio
>> exactly does), since the compatible you suggested matched with the TI
>> driver.
>
> First of all, please don't make assumptions. It's better to ask rather
> than making assumptions which turn up to be incorrect.
>
> Compatibles describe hardware. DT describes hardware. There are no
> drivers in question (yet).
> You have TI switch on your board, so you have to use "ti,hd3ss3220" to
> describe it.
>
> Existing schema describes it as an I2C device. You have to extend the
> schema to allow non-i2c attachment. Describe GPIOs, make reg optional.
> Make this description purely from the datasheet and usb-c-connector
> point of view.
>
>> If it was to add compatible in usb-conn-gpio, then we can support OTG
>> functionality with no schema changes I believe, but the compatible
>> string might need a different name to avoid clashing with the name in [2].
>
> And this is the second, largely independent question. The
> usb-conn-gpio driver is a platform driver.The existing hd3ss3220.c
> driver is an I2C one. There is no clash between them.
>
> Note, unlike plain gpio-b-connector, the switch supports more pins and
> actually provides USB-C information to the host even when used in the
> dumb mode. Thus it might be better to add a separate driver that
> registers typec port and reports USB-C events.
Hi Dmitry,
Regarding the comment:
"Note, unlike plain gpio-b-connector, the switch supports more pins and
actually provides USB-C information to the host even when used in the
dumb mode. Thus it might be better to add a separate driver that
registers typec port and reports USB-C events."
We are also aligned with your statement of expressing the hardware
correctly. Since this needs quite a bit of effort to write a new driver
for TI switch or modifying existing TI driver to add GPIO support, can't
we go ahead with peripheral support only since the driver support is
absent currently.
We will plan to submit the patches in upcoming days for this. Since we
usually enable USB in peripheral mode so that USB debug (adb) will work,
I am thinking we can merge this and take up the OTG/host mode
separately. Please let me know your feedback on this.
Thanks
Komal
>
>>
>> [1]:
>> https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
>>
>> [2]:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/hd3ss3220.c?h=v6.9-rc2
>>
>> Regards,
>> Krishna,
>>
>>>> * Link to v2: https://lore.kernel.org/linux-arm-msm/[email protected]/
>>>>
>>>> Changes in v2:
>>>> * Changes qmpphy node name
>>>> * Changes dr_mode to otg and added USB-B port USB role switch
>>>> * Dropped maximum-speed property from usb dwc3 node
>>>> * Link to v1: https://lore.kernel.org/linux-arm-msm/[email protected]/
>>>>
>>>> [1] https://lore.kernel.org/linux-arm-msm/[email protected]/
>>>> [2] https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
>>>> ------
>>>>
>>>> Komal Bajaj (3):
>>>> arm64: dts: qcom: qdu1000: Add USB3 and PHY support
>>>> arm64: dts: qcom: qdu1000-idp: enable USB nodes
>>>> arm64: dts: qcom: qru1000-idp: enable USB nodes
>>>>
>>>> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 23 +++++
>>>> arch/arm64/boot/dts/qcom/qdu1000.dtsi | 120 +++++++++++++++++++++++
>>>> arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++
>>>> 3 files changed, 166 insertions(+)
>>>>
>>>> --
>>>> 2.42.0
>>>>
>>>>
>>>
>>>
>
>
>
On Wed, 8 May 2024 at 09:12, Komal Bajaj <[email protected]> wrote:
>
>
>
> On 5/2/2024 5:14 PM, Dmitry Baryshkov wrote:
> > On Thu, 2 May 2024 at 12:48, Krishna Kurapati PSSNV
> > <[email protected]> wrote:
> >>
> >>
> >>
> >> On 5/2/2024 2:39 PM, Dmitry Baryshkov wrote:
> >>> On Thu, 2 May 2024 at 12:04, Komal Bajaj <[email protected]> wrote:
> >>>>
> >>>> This series adds devicetree nodes to support interconnects and usb for qdu/qru1000.
> >>>> This is based on previously sent driver series[1].
> >>>>
> >>>> ------
> >>>> Changes in v3:
> >>>> * As per comments on upstream[2], to get role-switch working on QDU/QRU1000, it was recommended to
> >>>> use the actual TI switch driver. Since driver doesn't have the functionality to provide role-switch
> >>>> based on gpio, thus reverting back USB dr_mode to peripheral and removed the remote end-point nodes
> >>>> and usb-conn-gpio based role switch functionality.
> >>>
> >>> This is not correct. The recommendation was to describe hardware properly.
> >>> Which means adding schema description, adding ti,your-switch
> >>> compatible to the usb-conn-gpio.c driver, etc.
> >>>
> >>
> >> Hi Dmitry,
> >>
> >> Sorry for the confusion. In the comments [1],
> >>
> >> "So the compatible string should be "ti,hd3ss3220". Which is fine to be
> >> used in the platform driver. Just describe the differences in the
> >> schema."
> >>
> >> The compatible "ti,hd3ss3220" is already associated with a TI switch
> >> driver [2]. But it works based on I2C. So we assumed you wanted us to
> >> make changes to [2] by adding GPIO functionality (which usb-conn-gpio
> >> exactly does), since the compatible you suggested matched with the TI
> >> driver.
> >
> > First of all, please don't make assumptions. It's better to ask rather
> > than making assumptions which turn up to be incorrect.
> >
> > Compatibles describe hardware. DT describes hardware. There are no
> > drivers in question (yet).
> > You have TI switch on your board, so you have to use "ti,hd3ss3220" to
> > describe it.
> >
> > Existing schema describes it as an I2C device. You have to extend the
> > schema to allow non-i2c attachment. Describe GPIOs, make reg optional.
> > Make this description purely from the datasheet and usb-c-connector
> > point of view.
> >
> >> If it was to add compatible in usb-conn-gpio, then we can support OTG
> >> functionality with no schema changes I believe, but the compatible
> >> string might need a different name to avoid clashing with the name in [2].
> >
> > And this is the second, largely independent question. The
> > usb-conn-gpio driver is a platform driver.The existing hd3ss3220.c
> > driver is an I2C one. There is no clash between them.
> >
> > Note, unlike plain gpio-b-connector, the switch supports more pins and
> > actually provides USB-C information to the host even when used in the
> > dumb mode. Thus it might be better to add a separate driver that
> > registers typec port and reports USB-C events.
>
> Hi Dmitry,
>
> Regarding the comment:
> "Note, unlike plain gpio-b-connector, the switch supports more pins and
> actually provides USB-C information to the host even when used in the
> dumb mode. Thus it might be better to add a separate driver that
> registers typec port and reports USB-C events."
>
> We are also aligned with your statement of expressing the hardware
> correctly. Since this needs quite a bit of effort to write a new driver
> for TI switch or modifying existing TI driver to add GPIO support, can't
> we go ahead with peripheral support only since the driver support is
> absent currently.
I think you are again mixing things here. You don't have to use the
existing TI driver. Instead you can start with the
gpio-b-usb-connector driver and later push a new driver for the switch
instead. This has nothing to do with the hardware description of the
device.
>
> We will plan to submit the patches in upcoming days for this. Since we
> usually enable USB in peripheral mode so that USB debug (adb) will work,
> I am thinking we can merge this and take up the OTG/host mode
> separately. Please let me know your feedback on this.
Well, granted that we are now close to the release, you can not merge
anything. So you have about three weeks before the next patches can
hit Bjorn's tree. I think this leaves plenty of time to develop a
proper solution.
>
> Thanks
> Komal
>
> >
> >>
> >> [1]:
> >> https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
> >>
> >> [2]:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/hd3ss3220.c?h=v6.9-rc2
> >>
> >> Regards,
> >> Krishna,
> >>
> >>>> * Link to v2: https://lore.kernel.org/linux-arm-msm/[email protected]/
> >>>>
> >>>> Changes in v2:
> >>>> * Changes qmpphy node name
> >>>> * Changes dr_mode to otg and added USB-B port USB role switch
> >>>> * Dropped maximum-speed property from usb dwc3 node
> >>>> * Link to v1: https://lore.kernel.org/linux-arm-msm/[email protected]/
> >>>>
> >>>> [1] https://lore.kernel.org/linux-arm-msm/[email protected]/
> >>>> [2] https://lore.kernel.org/all/CAA8EJppNZrLzT=vGS0NXnKJT_wL+bMB9jFhJ9K7b7FPgFQbcig@mail.gmail.com/
> >>>> ------
> >>>>
> >>>> Komal Bajaj (3):
> >>>> arm64: dts: qcom: qdu1000: Add USB3 and PHY support
> >>>> arm64: dts: qcom: qdu1000-idp: enable USB nodes
> >>>> arm64: dts: qcom: qru1000-idp: enable USB nodes
> >>>>
> >>>> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 23 +++++
> >>>> arch/arm64/boot/dts/qcom/qdu1000.dtsi | 120 +++++++++++++++++++++++
> >>>> arch/arm64/boot/dts/qcom/qru1000-idp.dts | 23 +++++
> >>>> 3 files changed, 166 insertions(+)
> >>>>
> >>>> --
> >>>> 2.42.0
> >>>>
> >>>>
> >>>
> >>>
> >
> >
> >
--
With best wishes
Dmitry