Message-ID: <bce19eaf-11bf-4ce8-bc15-dffb96cae9d4@freeshell.de>
Date: Thu, 23 Jan 2025 03:38:51 -0800
From: E Shattow <e@...eshell.de>
To: Minda Chen <minda.chen@...rfivetech.com>, Conor Dooley <conor@...nel.org>
Cc: Emil Renner Berthing <kernel@...il.dk>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] riscv: dts: starfive: jh7110: pciephy0 USB 3.0
 configuration registers



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@...eshell.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@...00000 {
>>>>>>>    		pciephy0: phy@...10000 {
>>>>>>>    			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

Powered by blists - more mailing lists