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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> 
> 
> 
> 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@xxxxxxxxxxxx>
> >>>>>>> ---
> >>>>>>>    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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux