[PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[E Shattow]

StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP block that
may exclusively use pciephy0 for USB3.0 connectivity. Add the register
offsets for the driver to enable/disable USB3.0 on pciephy0.

Signed-off-by: E Shattow <e@freeshell.de>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 0d8339357bad..75ff07303e8b 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
 		pciephy0: phy@10210000 {
 			compatible = "starfive,jh7110-pcie-phy";
 			reg = <0x0 0x10210000 0x0 0x10000>;
+			starfive,sys-syscon = <&sys_syscon 0x18>;
+			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
 			#phy-cells = <0>;
 		};
 
-- 
2.45.2

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP block that
> may exclusively use pciephy0 for USB3.0 connectivity. Add the register
> offsets for the driver to enable/disable USB3.0 on pciephy0.
> 
> Signed-off-by: E Shattow <e@freeshell.de>
> ---
>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 0d8339357bad..75ff07303e8b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
>  		pciephy0: phy@10210000 {
>  			compatible = "starfive,jh7110-pcie-phy";
>  			reg = <0x0 0x10210000 0x0 0x10000>;
> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;

Why weren't these added in the first place? Minda, do you know?

>  			#phy-cells = <0>;
>  		};
>  
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Minda Chen]

> 
> On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> > StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP block
> > that may exclusively use pciephy0 for USB3.0 connectivity. Add the
> > register offsets for the driver to enable/disable USB3.0 on pciephy0.
> >
> > Signed-off-by: E Shattow <e@freeshell.de>
> > ---
> >  arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > index 0d8339357bad..75ff07303e8b 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
> >  		pciephy0: phy@10210000 {
> >  			compatible = "starfive,jh7110-pcie-phy";
> >  			reg = <0x0 0x10210000 0x0 0x10000>;
> > +			starfive,sys-syscon = <&sys_syscon 0x18>;
> > +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> 
> Why weren't these added in the first place? Minda, do you know?
> 
The driver only require to set syscon register while the PHY attach to Cadence USB.(star64 board case)
The PHY default attach to PCIe0, VF2 board do not set any setting. So I don't set it.
> >  			#phy-cells = <0>;
> >  		};
> >
> > --
> > 2.45.2
> >

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
> 
> 
> > 
> > On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> > > StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP block
> > > that may exclusively use pciephy0 for USB3.0 connectivity. Add the
> > > register offsets for the driver to enable/disable USB3.0 on pciephy0.
> > >
> > > Signed-off-by: E Shattow <e@freeshell.de>
> > > ---
> > >  arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > index 0d8339357bad..75ff07303e8b 100644
> > > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
> > >  		pciephy0: phy@10210000 {
> > >  			compatible = "starfive,jh7110-pcie-phy";
> > >  			reg = <0x0 0x10210000 0x0 0x10000>;
> > > +			starfive,sys-syscon = <&sys_syscon 0x18>;
> > > +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> > 
> > Why weren't these added in the first place? Minda, do you know?
> > 
> The driver only require to set syscon register while the PHY attach to Cadence USB.(star64 board case)
> The PHY default attach to PCIe0, VF2 board do not set any setting. So I don't set it.

Does this mean that the change should be made in files where it will
only affect non-VF2 boards, or is it harmless if applied to the VF2
also?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Minda Chen]

> 
> On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
> >
> >
> > >
> > > On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> > > > StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP
> > > > block that may exclusively use pciephy0 for USB3.0 connectivity.
> > > > Add the register offsets for the driver to enable/disable USB3.0 on
> pciephy0.
> > > >
> > > > Signed-off-by: E Shattow <e@freeshell.de>
> > > > ---
> > > >  arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > index 0d8339357bad..75ff07303e8b 100644
> > > > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
> > > >  		pciephy0: phy@10210000 {
> > > >  			compatible = "starfive,jh7110-pcie-phy";
> > > >  			reg = <0x0 0x10210000 0x0 0x10000>;
> > > > +			starfive,sys-syscon = <&sys_syscon 0x18>;
> > > > +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> > >
> > > Why weren't these added in the first place? Minda, do you know?
> > >
> > The driver only require to set syscon register while the PHY attach to
> > Cadence USB.(star64 board case) The PHY default attach to PCIe0, VF2 board
> do not set any setting. So I don't set it.
> 
> Does this mean that the change should be made in files where it will only affect
> non-VF2 boards, or is it harmless if applied to the VF2 also?
Harmless. The PCIe PHY driver still set the PCIe mode syscon setting.

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[E Shattow]

Hi Minda,

On 1/15/25 02:58, Minda Chen wrote:
> 
> 
>>
>> On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
>>>
>>>
>>>>
>>>> On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
>>>>> StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP
>>>>> block that may exclusively use pciephy0 for USB3.0 connectivity.
>>>>> Add the register offsets for the driver to enable/disable USB3.0 on
>> pciephy0.
>>>>>
>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>> ---
>>>>>   arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
>>>>>   1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>> b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>> index 0d8339357bad..75ff07303e8b 100644
>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>> @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
>>>>>   		pciephy0: phy@10210000 {
>>>>>   			compatible = "starfive,jh7110-pcie-phy";
>>>>>   			reg = <0x0 0x10210000 0x0 0x10000>;
>>>>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>>>>> +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
>>>>
>>>> Why weren't these added in the first place? Minda, do you know?
>>>>
>>> The driver only require to set syscon register while the PHY attach to
>>> Cadence USB.(star64 board case) The PHY default attach to PCIe0, VF2 board
>> do not set any setting. So I don't set it.
>>
>> Does this mean that the change should be made in files where it will only affect
>> non-VF2 boards, or is it harmless if applied to the VF2 also?
> Harmless. The PCIe PHY driver still set the PCIe mode syscon setting.

Sounds good to me. However some tangent topic related to this series:

Our questions and answers in this discussion are a representation of 
what is missing from the documentation.

What do I want to know? :  "pdrstn split sw usbpipe plugen" abbreviation.

What are the full words that is from?

I will guess the words are:

"Power domain reset negative? Split... Switch? USB pipeline plug enable?"

When this is explained for me I will send a patch to add information 
into documentation at dt-bindings/phy/starfive,jh7110-pcie-phy.yaml 
file. I know that the functionality is already said in discussion;  What 
I want are the full words to expand the "pdrstn split sw usbpipe plugen" 
as any abbreviation would also be expanded and explained in documentation.

It would be difficult to improve the documentation before our discussion 
about this series here. Now it is clear what questions and answers are 
missing from documentation.

-E

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Minda Chen]

> 
> Hi Minda,
> 
> On 1/15/25 02:58, Minda Chen wrote:
> >
> >
> >>
> >> On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
> >>>
> >>>
> >>>>
> >>>> On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> >>>>> StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP
> >>>>> block that may exclusively use pciephy0 for USB3.0 connectivity.
> >>>>> Add the register offsets for the driver to enable/disable USB3.0
> >>>>> on
> >> pciephy0.
> >>>>>
> >>>>> Signed-off-by: E Shattow <e@freeshell.de>
> >>>>> ---
> >>>>>   arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
> >>>>>   1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>> b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>> index 0d8339357bad..75ff07303e8b 100644
> >>>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>> @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
> >>>>>   		pciephy0: phy@10210000 {
> >>>>>   			compatible = "starfive,jh7110-pcie-phy";
> >>>>>   			reg = <0x0 0x10210000 0x0 0x10000>;
> >>>>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> >>>>> +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> >>>>
> >>>> Why weren't these added in the first place? Minda, do you know?
> >>>>
> >>> The driver only require to set syscon register while the PHY attach
> >>> to Cadence USB.(star64 board case) The PHY default attach to PCIe0,
> >>> VF2 board
> >> do not set any setting. So I don't set it.
> >>
> >> Does this mean that the change should be made in files where it will
> >> only affect
> >> non-VF2 boards, or is it harmless if applied to the VF2 also?
> > Harmless. The PCIe PHY driver still set the PCIe mode syscon setting.
> 
> Sounds good to me. However some tangent topic related to this series:
> 
> Our questions and answers in this discussion are a representation of what is
> missing from the documentation.
> 
> What do I want to know? :  "pdrstn split sw usbpipe plugen" abbreviation.
> 
> What are the full words that is from?
> 
> I will guess the words are:
> 
> "Power domain reset negative? Split... Switch? USB pipeline plug enable?"
> 
> When this is explained for me I will send a patch to add information into
> documentation at dt-bindings/phy/starfive,jh7110-pcie-phy.yaml
> file. I know that the functionality is already said in discussion;  What I want are
> the full words to expand the "pdrstn split sw usbpipe plugen"
> as any abbreviation would also be expanded and explained in documentation.
> 
> It would be difficult to improve the documentation before our discussion about
> this series here. Now it is clear what questions and answers are missing from
> documentation.
> 
> -E
In my view, pdrstn split sw usbpipe is bit17 setting. Set to 1 is mean split the PCIe PHY from
Cadence USB controller.

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[E Shattow]

On 1/22/25 02:41, Minda Chen wrote:
> 
> 
> 
>>
>> Hi Minda,
>>
>> On 1/15/25 02:58, Minda Chen wrote:
>>>
>>>
>>>>
>>>> On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
>>>>>>> StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP
>>>>>>> block that may exclusively use pciephy0 for USB3.0 connectivity.
>>>>>>> Add the register offsets for the driver to enable/disable USB3.0
>>>>>>> on
>>>> pciephy0.
>>>>>>>
>>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>>>> ---
>>>>>>>    arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
>>>>>>>    1 file changed, 2 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>>>> b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>>>> index 0d8339357bad..75ff07303e8b 100644
>>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>>>>> @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
>>>>>>>    		pciephy0: phy@10210000 {
>>>>>>>    			compatible = "starfive,jh7110-pcie-phy";
>>>>>>>    			reg = <0x0 0x10210000 0x0 0x10000>;
>>>>>>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>>>>>>> +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
>>>>>>
>>>>>> Why weren't these added in the first place? Minda, do you know?
>>>>>>
>>>>> The driver only require to set syscon register while the PHY attach
>>>>> to Cadence USB.(star64 board case) The PHY default attach to PCIe0,
>>>>> VF2 board
>>>> do not set any setting. So I don't set it.
>>>>
>>>> Does this mean that the change should be made in files where it will
>>>> only affect
>>>> non-VF2 boards, or is it harmless if applied to the VF2 also?
>>> Harmless. The PCIe PHY driver still set the PCIe mode syscon setting.
>>
>> Sounds good to me. However some tangent topic related to this series:
>>
>> Our questions and answers in this discussion are a representation of what is
>> missing from the documentation.
>>
>> What do I want to know? :  "pdrstn split sw usbpipe plugen" abbreviation.
>>
>> What are the full words that is from?
>>
>> I will guess the words are:
>>
>> "Power domain reset negative? Split... Switch? USB pipeline plug enable?"
>>
>> When this is explained for me I will send a patch to add information into
>> documentation at dt-bindings/phy/starfive,jh7110-pcie-phy.yaml
>> file. I know that the functionality is already said in discussion;  What I want are
>> the full words to expand the "pdrstn split sw usbpipe plugen"
>> as any abbreviation would also be expanded and explained in documentation.
>>
>> It would be difficult to improve the documentation before our discussion about
>> this series here. Now it is clear what questions and answers are missing from
>> documentation.
>>
>> -E
> In my view, pdrstn split sw usbpipe is bit17 setting. Set to 1 is mean split the PCIe PHY from
> Cadence USB controller.

Hi, Minda. Yes, the functional description is very good.

What I want to know is the language "pdrstn" for example, was this from 
StarFive and someone you can ask what those words are ? Else is it from 
Cadence and I should next ask some design person from Cadence company? I 
want to show in documentation what is the long word (or many words) that 
are changed to short words and become "pdrstn split sw usbpipe plugen".

When I read "pdrstn split sw usbpipe plugen":

pdrstn = ?  I do not know, it is different than any word I know. This 
could be like Power Domain (or Power Delivery), Reset, Negative... Pulse 
Data Rate Standard... Plug Drivers Transmission... it is non-sense to 
guess. I prefer in writing documentation to give some information that 
is accurate.

split = split. Ok. I think this is true. You provide us code and good 
description that bit17 setting is a "split" action so this is easy for 
understanding.

sw = ? maybe this is "switch"?  or "software", "southwest", "signal 
watch", "sine wave", ... probably switch.

usbpipe = 'pipe' is a connection. okay, this is acceptable. It is a 
"pipe" connection of Cadence IP block with a different part of JH7110 
design (PCIe?)

plugen = plug enable? I do not think of any different possible words for 
this, so it may be that.

I am aware this request is not any better for us to understand the code. 
We do know what the code does - Thank you, I appreciate your time! The 
English word choices are not very interesting (?) but I want to be 
accurate for documentation.

Someone at a moment in past time decided "pdrstn split sw usbpipe 
plugen" is a good description for this. Who is that person? What are 
those long words they did change into confusing short form? :-)

If you can ask around and maybe someone at StarFive does know?  Else you 
can confirm that it was something "not documented" and I will explain in 
documentation that it was "not documented" this exact source of words 
for "pdrstn split sw usbpipe plugen". We can substitute a functional 
description with or without a source of the words.

Best regards,

E Shattow

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Minda Chen]

> 
> 
> 
> On 1/22/25 02:41, Minda Chen wrote:
> >
> >
> >
> >>
> >> Hi Minda,
> >>
> >> On 1/15/25 02:58, Minda Chen wrote:
> >>>
> >>>
> >>>>
> >>>> On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> >>>>>>> StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP
> >>>>>>> block that may exclusively use pciephy0 for USB3.0 connectivity.
> >>>>>>> Add the register offsets for the driver to enable/disable USB3.0
> >>>>>>> on
> >>>> pciephy0.
> >>>>>>>
> >>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
> >>>>>>> ---
> >>>>>>>    arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
> >>>>>>>    1 file changed, 2 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>>>> b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>>>> index 0d8339357bad..75ff07303e8b 100644
> >>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >>>>>>> @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
> >>>>>>>    		pciephy0: phy@10210000 {
> >>>>>>>    			compatible = "starfive,jh7110-pcie-phy";
> >>>>>>>    			reg = <0x0 0x10210000 0x0 0x10000>;
> >>>>>>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> >>>>>>> +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> >>>>>>
> >>>>>> Why weren't these added in the first place? Minda, do you know?
> >>>>>>
> >>>>> The driver only require to set syscon register while the PHY
> >>>>> attach to Cadence USB.(star64 board case) The PHY default attach
> >>>>> to PCIe0,
> >>>>> VF2 board
> >>>> do not set any setting. So I don't set it.
> >>>>
> >>>> Does this mean that the change should be made in files where it
> >>>> will only affect
> >>>> non-VF2 boards, or is it harmless if applied to the VF2 also?
> >>> Harmless. The PCIe PHY driver still set the PCIe mode syscon setting.
> >>
> >> Sounds good to me. However some tangent topic related to this series:
> >>
> >> Our questions and answers in this discussion are a representation of
> >> what is missing from the documentation.
> >>
> >> What do I want to know? :  "pdrstn split sw usbpipe plugen" abbreviation.
> >>
> >> What are the full words that is from?
> >>
> >> I will guess the words are:
> >>
> >> "Power domain reset negative? Split... Switch? USB pipeline plug enable?"
> >>
> >> When this is explained for me I will send a patch to add information
> >> into documentation at dt-bindings/phy/starfive,jh7110-pcie-phy.yaml
> >> file. I know that the functionality is already said in discussion;
> >> What I want are the full words to expand the "pdrstn split sw usbpipe
> plugen"
> >> as any abbreviation would also be expanded and explained in
> documentation.
> >>
> >> It would be difficult to improve the documentation before our
> >> discussion about this series here. Now it is clear what questions and
> >> answers are missing from documentation.
> >>
> >> -E
> > In my view, pdrstn split sw usbpipe is bit17 setting. Set to 1 is mean
> > split the PCIe PHY from Cadence USB controller.
> 
> Hi, Minda. Yes, the functional description is very good.
> 
> What I want to know is the language "pdrstn" for example, was this from
> StarFive and someone you can ask what those words are ? Else is it from
> Cadence and I should next ask some design person from Cadence company? I
> want to show in documentation what is the long word (or many words) that are
> changed to short words and become "pdrstn split sw usbpipe plugen".
> 
> When I read "pdrstn split sw usbpipe plugen":
> 
> pdrstn = ?  I do not know, it is different than any word I know. This could be like
> Power Domain (or Power Delivery), Reset, Negative... Pulse Data Rate
> Standard... Plug Drivers Transmission... it is non-sense to guess. I prefer in
> writing documentation to give some information that is accurate.
> 
> split = split. Ok. I think this is true. You provide us code and good description that
> bit17 setting is a "split" action so this is easy for understanding.
> 
> sw = ? maybe this is "switch"?  or "software", "southwest", "signal watch",
> "sine wave", ... probably switch.
> 
> usbpipe = 'pipe' is a connection. okay, this is acceptable. It is a "pipe" connection
> of Cadence IP block with a different part of JH7110 design (PCIe?)
> 
> plugen = plug enable? I do not think of any different possible words for this, so it
> may be that.
> 
> I am aware this request is not any better for us to understand the code.
> We do know what the code does - Thank you, I appreciate your time! The
> English word choices are not very interesting (?) but I want to be accurate for
> documentation.
> 
> Someone at a moment in past time decided "pdrstn split sw usbpipe plugen" is a
> good description for this. Who is that person? What are those long words they
> did change into confusing short form? :-)
> 
> If you can ask around and maybe someone at StarFive does know?  Else you can
> confirm that it was something "not documented" and I will explain in
> documentation that it was "not documented" this exact source of words for
> "pdrstn split sw usbpipe plugen". We can substitute a functional description with
> or without a source of the words.
> 
I think set it to "not documented" is Okay. The JH7110 USB system Designer
has left from StarFive. So I cant give more detail about this.

> Best regards,
> 
> E Shattow

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

On Wed, Jan 15, 2025 at 10:58:39AM +0000, Minda Chen wrote:
> 
> 
> > 
> > On Tue, Jan 14, 2025 at 05:42:28AM +0000, Minda Chen wrote:
> > >
> > >
> > > >
> > > > On Thu, Jan 02, 2025 at 10:37:36AM -0800, E Shattow wrote:
> > > > > StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP
> > > > > block that may exclusively use pciephy0 for USB3.0 connectivity.
> > > > > Add the register offsets for the driver to enable/disable USB3.0 on
> > pciephy0.
> > > > >
> > > > > Signed-off-by: E Shattow <e@freeshell.de>
> > > > > ---
> > > > >  arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > > index 0d8339357bad..75ff07303e8b 100644
> > > > > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > > > @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
> > > > >  		pciephy0: phy@10210000 {
> > > > >  			compatible = "starfive,jh7110-pcie-phy";
> > > > >  			reg = <0x0 0x10210000 0x0 0x10000>;
> > > > > +			starfive,sys-syscon = <&sys_syscon 0x18>;
> > > > > +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> > > >
> > > > Why weren't these added in the first place? Minda, do you know?
> > > >
> > > The driver only require to set syscon register while the PHY attach to
> > > Cadence USB.(star64 board case) The PHY default attach to PCIe0, VF2 board
> > do not set any setting. So I don't set it.
> > 
> > Does this mean that the change should be made in files where it will only affect
> > non-VF2 boards, or is it harmless if applied to the VF2 also?
> Harmless. The PCIe PHY driver still set the PCIe mode syscon setting.

Okay, I'll pick this up after the merge window, pending an Ack from
Emil.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers

[Emil Renner Berthing]

E Shattow wrote:
> StarFive JH7110 contains a Cadence USB2.0+USB3.0 controller IP block that
> may exclusively use pciephy0 for USB3.0 connectivity. Add the register
> offsets for the driver to enable/disable USB3.0 on pciephy0.
>
> Signed-off-by: E Shattow <e@freeshell.de>

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Thanks!

> ---
>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 0d8339357bad..75ff07303e8b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -611,6 +611,8 @@ usbphy0: phy@10200000 {
>  		pciephy0: phy@10210000 {
>  			compatible = "starfive,jh7110-pcie-phy";
>  			reg = <0x0 0x10210000 0x0 0x10000>;
> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> +			starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
>  			#phy-cells = <0>;
>  		};
>
> --
> 2.45.2
>

[PATCH v1 2/2] riscv: dts: starfive: jh7110-pine64-star64: enable USB 3.0 port

[E Shattow]

One of four USB-A ports on the Pine64 Star64 is USB 3.0 which requires to
disable PCIE0 and change the mode of PCIE0 PHY to USB3.0 operation. The
remaining three USB-A ports are USB 2.0 with the USB0 PHY and do not
conflict with any of PCIE0 or PCIE1. PCIE1 (1-lane) routes to a PCIe X4
connector.

Signed-off-by: E Shattow <e@freeshell.de>
---
 arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
index b764d4d92fd9..31e825be2065 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
@@ -100,3 +100,8 @@ &usb0 {
 	pinctrl-0 = <&usb0_pins>;
 	status = "okay";
 };
+
+&usb_cdns3 {
+	phys = <&usbphy0>, <&pciephy0>;
+	phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
+};
-- 
2.45.2

Re: [PATCH v1 2/2] riscv: dts: starfive: jh7110-pine64-star64: enable USB 3.0 port

[Emil Renner Berthing]

E Shattow wrote:
> One of four USB-A ports on the Pine64 Star64 is USB 3.0 which requires to
> disable PCIE0 and change the mode of PCIE0 PHY to USB3.0 operation. The
> remaining three USB-A ports are USB 2.0 with the USB0 PHY and do not
> conflict with any of PCIE0 or PCIE1. PCIE1 (1-lane) routes to a PCIe X4
> connector.
>
> Signed-off-by: E Shattow <e@freeshell.de>

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Thanks!

> ---
>  arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
> index b764d4d92fd9..31e825be2065 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
> @@ -100,3 +100,8 @@ &usb0 {
>  	pinctrl-0 = <&usb0_pins>;
>  	status = "okay";
>  };
> +
> +&usb_cdns3 {
> +	phys = <&usbphy0>, <&pciephy0>;
> +	phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
> +};
> --
> 2.45.2
>

Re: [PATCH v1 0/2] riscv: dts: starfive: Enable USB3.0 for Pine64 Star64

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

On Thu, 02 Jan 2025 10:37:35 -0800, E Shattow wrote:
> Add PCIe configuration of USB3.0 registers for pciephy0 mode on all
> JH7110 targets:
> 
> VisionFive2 1.2a, not applicable (USB-C peripheral mode power input)
> VisionFive2 1.3b, not applicable (USB-C peripheral mode power input)
> Milk-V Mars, one USB-A port USB3.0 disable (USB2.0 + PCIe0 peripheral)
> Pine64 Star64, one USB-A port USB3.0 enable (USB3.0 + PCIe0 disable)
> 
> [...]

Applied to riscv-dt-for-next, thanks!

[1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0 configuration registers
      https://git.kernel.org/conor/c/65e8b9912670
[2/2] riscv: dts: starfive: jh7110-pine64-star64: enable USB 3.0 port
      https://git.kernel.org/conor/c/38818f7c9c17

Thanks,
Conor.