ChanServ changed the topic of #linux-msm to:
marvin24_ has joined #linux-msm
marvin24 has quit [Ping timeout: 480 seconds]
animist2 has quit [Remote host closed the connection]
animist2 has joined #linux-msm
deathmist has quit [Ping timeout: 480 seconds]
fossdd has quit [Remote host closed the connection]
fossdd has joined #linux-msm
jhovold has joined #linux-msm
smpl has joined #linux-msm
smpl has quit [Remote host closed the connection]
smpl has joined #linux-msm
deathmist has joined #linux-msm
pespin has joined #linux-msm
fossdd has quit [Remote host closed the connection]
fossdd has joined #linux-msm
<narmstrong> strongtz[m]: try adding the MCLKs https://termbin.com/38xu and enable them from the sound card like the IBI, I have some data on the I2S1 lines, but I need to use PRIMARY_MI2S_RX... no idea which id drivers the i2s0 lines
<strongtz[m]> narmstrong: uh, you mean you're getting data on secondary mi2s by setting primary i2s everywhere?
<strongtz[m]> I do need the secondary i2s though
<narmstrong> strongtz[m]: hmm, yeah, so you're using the i2s1 pins on gpio 121/122/123 right ?
<narmstrong> let me try to do the same on sm8550-hdk
<strongtz[m]> narmstrong: right, my current device looks like this: https://paste.armbian.com/opasezuciz.cpp
<strongtz[m]> haven't added the MCLKs
mgonzalez has joined #linux-msm
<mgonzalez> Is there a preferred place to add status = "disabled"; in a DTSI device node? First line? Last line? After compatible? After reg? Anywhere?
anuw[m] has joined #linux-msm
<anuw[m]> hello, after some struggle with sm7125 csiphy and clks, seemed i can capture a frame-000000.bin file by yavta, how can i examine the content like rgb? here is the file uploaded for reference https://filebin.net/bjmuxa82jn34ie3e
<mgonzalez> lumag: can I extend Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml for qcom,hdmi-phy-8998 ?
<jhovold> mgonzalez: status properties always goes last (i.e. just before any potential child nodes)
<mgonzalez> jhovold: thanks for the clarification!
<lumag> mgonzalez, yes, that's the point
<mgonzalez> jhovold: looking at msm8998.dtsi it looks like this recommendation was not followed for many nodes
<jhovold> there's some inconsistency in older dtsis
<mgonzalez> that makes sense
<lumag> mgonzalez, for older DT patches are welcome and appreciated. We have been going through some of the historical luggage, but the debt is too high
<mgonzalez> lumag: even a patch just moving the status around to the proper place?
<lumag> that depends on bamse and konradybcio.
<lumag> If you are touching the node in any way, then yes.
<mgonzalez> Not touching old nodes, writing new ones, and confused by random placement of status prop.
<lumag> my 2c would be that the answer still should be 'yes'.
<lumag> but I'd let maintainers to speak for them.
<mgonzalez> I know there was some - vs _ churn earlier
<mgonzalez> I'm ambivalent wrt churn
<bamse> the [-_] problem is well defined, so corrections there are appreciated...as is fixes to align with dt bindings to squash validation errors
<mgonzalez> bamse: moving the status prop would not squash validation errors though, IIUC
<bamse> when it comes to just chaning the order of properties...i'm not as enthusiastic when accepting those...i would like us to have a tool providing a well defined definition of "correct order"
<mgonzalez> that makes sense
<mgonzalez> wait for something more formally defined
<lumag> bamse, I fear that when we have such a tool, we are going to see lots of strange patches from random people.
<lumag> Compare that with random patches using __free(of_node). Which in some cases break stuff
<bamse> lumag: i fear that too, but at least we don't have arbitrary follow-up patches
<lumag> ?
<bamse> lumag: i hate the _free() stuff...it's not idiomatic kernel code and people are sending patches converting perfectly working code because "it's better"
<bamse> lumag: if we have a definition of ordering and a tool that validates that, we should have one round of churn to align with that
<mgonzalez> until someone changes the tool :p
<bamse> lumag: in contrast to people sending patches shuffling things based on gut feeling
<lumag> :-D
<mgonzalez> bamse, lumag: I would think a node should be written in the same order as defined in the binding?
<lumag> mgonzalez, do you mean the properties? Well, I never figured that out.
<mgonzalez> for example, Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml specifies clocks,clock-names,power-domains,vcca-supply,vddio-supply,#clock-cells,#phy-cells
<mgonzalez> it feels a bit weird to have #clock-cells,#phy-cells this far from the clocks props...
<mgonzalez> Or maybe the # props are expected to be at the end in a binding spec
<bamse> mgonzalez: but clocks is input signals, #clock-cells defines the outputs
<mgonzalez> ah, then my remark is bogus :p
<mgonzalez> I'll just follow the binding spec & call it a day then
<mgonzalez> whoever wrote that (Rob) knows infinitely more than me
<bamse> and no, the yaml files has not been written to convey the order of propertiess...so that wouldn't work (today)
<mgonzalez> arg!
<bamse> mgonzalez: Documentation/devicetree/bindings/dts-coding-style.rst <- gives you some guidance
<mgonzalez> Thanks for the pointer, will review it
<mgonzalez> This is one of the biggest problems with Linux IMO
<bamse> that the users doesn't read the docs? ;P
<mgonzalez> No, that there is a huge amount of code, and that the "right way" is figured out after several iterations, at which point no one wants to fix the old stuff
<mgonzalez> A few years ago, I was writing an IRQ handler driver, I picked the last one that had been reviewed, and still I got torched for "not following the recommendations"
<mgonzalez> I think after a while, maintainers forget the learning curve they went through, and just don't understand why noobs are struggling
<mgonzalez> lumag: something simple like that: https://paste.debian.net/1318479/
<lumag> mgonzalez, which clock is ref_src?
<lumag> Inserting it in the middle would break it for msm8996, which has just three clocks.
<mgonzalez> <&rpmcc RPM_SMD_LN_BB_CLK1> ?
<mgonzalez> Will change the order and post an RFC patch
smpl has quit [Ping timeout: 480 seconds]
<mgonzalez> Actually, I didn't understand your remark about clks order
<mgonzalez> Aren't clock-names used to look up clocks by name?
<mgonzalez> And if 8996 & 8998 have different drivers, why is adding a clock for 8998 an issue for 8996?
<lumag> mgonzalez, because you are adding it to the common bindings block
<lumag> not to the driver-specific one
<lumag> mgonzalez, also, I see that the clock is being referenced in msm8998-mdss-pll.dtsi, but is it actually used?
<lumag> I can't find a reference in the msm-4.4 tree
<lumag> maybe jhugo or sboyd remember the 8998 clock tree. But I'd just assume that the GCC_HDMI_CLKREF_CLK is being fed by CLK_LN_BB1_CLK.
<lumag> If that's the case, you'd need to fix the GCC bindings and pass the CLK_LN_BB1_CLK to gcc.
<lumag> konradybcio, maybe you know ^^ ?
pespin has quit []
<sboyd> lumag: if those clocks are branches that never get turned off it's fine to remove them and just turn them on once at probe time. Is that the case there? If that's happening on other SoCs it is simpler to just turn them on once and then remove them from the array so that gpu driver gets NULL clks that do nothing.
<sboyd> lumag: I don't know but I would guess `GCC_HDMI_CLKREF_CLK` is fed by some internal branch on the XO pin that is wired to `CLK_LN_BB1_CLK`
<jhugo> I don't see any hdmi clocks listed in the gcc clock documentation
<bamse> jhugo: that's because "clkrefs aren't real clocks" (or something like that)...you often find them mentioned in the hpg though
<jhugo> bamse: yeah, but the HPG typically won't detail how they are sourced...
<bamse> jhugo: except for the clkrefs...on most platforms
<sboyd> sometimes the parent hierarchy was broken by not specifying the parent so that XO shutdown can still happen
<bamse> the good old "trust me bruh!" design pattern
<sboyd> I have no idea if that's the case here
<lumag> bamse, sboyd, jhugo: so for mgonzalez's case of msm8998 / hdmi, should we keep any additional clock on for the GCC_HDMI_CLKREF_CLK? Or having SMD_XO_CLK is enough?
<sboyd> I suspect SMD_XO_CLK is enough because RPM is the one controlling CLK_LN_BB1_CLK for the XO pin on the SoC
<jhugo> SMD_XO_CLK should be good
<lumag> mgonzalez, that probably means that you can safely ignore ref_clk_src
<lumag> sboyd, jhugo thanks!
<lumag> (and bamse of course, thank you)
jhovold has quit [Ping timeout: 480 seconds]