<strongtz[m]>
narmstrong: For me it's on secondary i2s. And I've tried adding clocks to dt and machine driver like yours, but it still doesn't work as before.
<strongtz[m]>
after amixer -c 0 cset iface=MIXER,name='SECONDARY_MI2S_RX Audio Mixer MultiMedia2' 1, and aplay -d 3 -D plughw:0,1 Summer.wav, it just hangs
<strongtz[m]>
if everything works fine, aplay should stop after 3 sec. Something seems to be stuck in the kernel
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
marvin24 has joined #linux-msm
marvin24_ has quit [Ping timeout: 480 seconds]
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Remote host closed the connection]
matalama has joined #linux-msm
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
matalama has quit [Ping timeout: 480 seconds]
matalama has joined #linux-msm
matalama has quit [Quit: Quit]
jhovold has joined #linux-msm
pespin has joined #linux-msm
<narmstrong>
strongtz[m]: speaker-test gets stuck for me at 48KHz, but it continues with 44.1KHz, did you probe the I/Os on the board to check if there's some signals
<aka_[m]>
konradybcio: i see you are running all over the place now
<aka_[m]>
X elite
ungeskriptet is now known as Guest7835
ungeskriptet has joined #linux-msm
Guest7835 has quit [Ping timeout: 480 seconds]
barni2000[m] has joined #linux-msm
barni2000[m] has left #linux-msm [#linux-msm]
<marc|gonzalez>
I don't understand the rationale for marking most of the large IP blocks as "disabled" in DTSI, when they don't require any board-specific configuration...
<marc|gonzalez>
stuff like the GPU, PCIe, sometimes USB, SD, UFS, etc
<aka_[m]>
SD could be unwired, UFS might not be also wired, GPU you might want to run board headless
<aka_[m]>
for gpu quite often its disabled because it might malfunction at current state
<aka_[m]>
also you need custom supplies sometimes
<aka_[m]>
and supplies are defined per-board unless its power-domain
mripard has joined #linux-msm
<krzk>
marc|gonzalez: SD, UFS - all depend on what is present on the board. PCI for embedded or finished devices (not development kits) actually as well.
<krzk>
USB depends on actual physical interfaces (ports) present on the board, sp... the same
<krzk>
so the only, only one from above list in general which could be enabled in DTSI is GPU... but are you sure it does not have board-specific configuratiob?
<krzk>
I think I never saw GPU without external supplies (so this would mean that supplies are coming from within the soc!)
<krzk>
Care to show any example?
<travmurav[m]>
gpu on modern qcom probably should be disabled by default because signed zap shader
<krzk>
yep, one more external resource
<travmurav[m]>
so even if it was fully generically supplied, would still be easily sad
<marc|gonzalez>
krzk: travmurav[m]: yeah for example adreno_gpu on msm8998, to enable it on my board, I just have status = "ok" and a zap-shader memory region
<marc|gonzalez>
I see also &gpu_mem which feels not very hardware-y
<krzk>
hm, actually I was wrong about gpu - most adreno's have internal supply
<marc|gonzalez>
all these memory regions definitions are just software settings :/
<krzk>
but then it is the only case from above list
<marc|gonzalez>
but maybe software settings in firmware = should be in DT
<marc|gonzalez>
krzk: to enable pcie0 and pcie_phy I just have status = "ok"
<krzk>
no clue what you mean by software settings, but as I said about PCI - this is board specific.
<krzk>
PCI cannot work without PCI slot or PCI devices
<krzk>
PCI devices are not present in the SoC itself (except maybe some packaged with WiFi)
<marc|gonzalez>
A host controller doesn't need any devices
<krzk>
so why something which cannot work or is meaningless to work should be enabled
<krzk>
host controller without devices is useless
<marc|gonzalez>
Device tree defines what is there, not what is useful
<krzk>
?
<marc|gonzalez>
&usb3 also just status = "ok"
<krzk>
Devicetree represents hardware. There is no finished PCI iin the soc. You must have a device.
<krzk>
marc|gonzalez: I explained above why USB must be like this...
<marc|gonzalez>
but &usb3phy has some supplies, so maybe controller without phy is useless
<krzk>
marc|gonzalez: do you understand the logic behind DTSI and DTS split?
<marc|gonzalez>
OK so if IP block cannot be used, it is marked disabled, got it.
<marc|gonzalez>
DTSI is what is provided by the SoC. DTS is what is board specific
<krzk>
If the SoC part is incomplete, then it is disabled
<marc|gonzalez>
I don't think the way it is done is rational
<krzk>
PCI is incomplete without devices because it is meaningless by itself, that's why it is disabled.
<krzk>
Just like SD is meaningless without actual memory or slot
<marc|gonzalez>
When 15 boards cable a regulator the same way as recommended by the reference design, the DTSI should code the reference design recommendation, and boards that deviate would over-rule the change
<marc|gonzalez>
OK, thanks for explaining the rationale.
<marc|gonzalez>
If there's a configuration in which it cannot be used => disabled in the DTSI
<krzk>
following your logic, all SPI and I2C should be enabled
<marc|gonzalez>
What about a video decoder?
<javierm>
marc|gonzalez: if one can factor out similar dts(i) snippets because most boards used a reference design, then you could have a separate .dtsi and could enable some DT nodes there
<javierm>
marc|gonzalez: but the SoC dtsi, shouldn't really enable IP blocks IMO as others mentioned
<krzk>
marc|gonzalez: depends whether board needs to provide anything. video codec is not a bus, so the arguments of USB/PCI/SD do not apply here, so it is just the case of firmware or supplies
<krzk>
but to bring analogy - SoC codecs are enabled
<marc|gonzalez>
Wouldn't it make sense for "disabled" to be the default state then?
<krzk>
marc|gonzalez: why? provide argument
<marc|gonzalez>
I will count to have objective numbers, but it looks like all useful nodes in msm8998.dtsi are disabled. Meaning disabled is in fact a de facto "default" state.
<krzk>
marc|gonzalez: that's not an argument. Sorry, DTSI is not organized on "de facto default state"
<krzk>
I described before the rules what is enabled and disabled. There is no rule "default state".
<marc|gonzalez>
Of course there is
<marc|gonzalez>
Default state is enabled.
<marc|gonzalez>
Your claim makes no sense?
<marc|gonzalez>
I am talking DT syntax here
<krzk>
What is that argument for?
<krzk>
Venus should be disabled because default DT syntax is "enabled"? What?
<marc|gonzalez>
Wow
<javierm>
marc|gonzalez: default state to be enabled makes sense to me, the SoC dtsi explicitly disables the IP blocks that can't be used standalone without other nodes defined by the boards dts
<krzk>
marc|gonzalez: now you change the topic silently to talk about DT spec... but we were talking about coding style before (as expressed in DTS coding style), so yeah, you can get people confused.
<krzk>
I guess the main concept is clear now, so I'll pass.
<marc|gonzalez>
krzk: discussing with you is really frustrating.
<marc|gonzalez>
javierm: it's like "static" in C. It's not the default, but the designers later regretted that decision. If most of the nodes have to be manually marked as disabled, it would seem to make sense to have it be default, do you see what I mean?
<javierm>
marc|gonzalez: but that's how is defined in the spec
<javierm>
"The lack of a status property should be
<javierm>
treated as if the property existed with the value of "okay"
<mripard>
marc|gonzalez: and just like static, it doesn't make sense to change it because it would inflict a huge pain to everyone for dubious results
<marc|gonzalez>
mripard: I am not proposing that, I understand
<javierm>
marc|gonzalez: but regardless of the spec, what I meant is that for me it does makes sense
<marc|gonzalez>
I am wondering if the spec designers feel the same way as the C designers
<marc|gonzalez>
Or if it's just expected to mark most of the DTSI nodes as disabled
<mripard>
does it matter what the opinion of the DT spec authors is?
<mripard>
it's what we have, it's what we must use
<marc|gonzalez>
:)
<travmurav[m]>
dt spec wrt status property makes sense and is indeed the correct default most of the time IMO, dtsi IP blocks being disabled are explicit, which is good imo (=="please pay attention") and fwiw not that common than you'd think
<mripard>
and the fact that "most" nodes are marked as disabled might be true for some SoCs, but it would definitely need some figures to back that claim.
<marc|gonzalez>
Understood. Thanks to all for providing perspective.
<javierm>
travmurav[m]: exactly, it makes the disable explicit which I believe is the correct thing to do
<mripard>
javierm: yeah, me too, especially when you consider the DT history
<travmurav[m]>
like, no status => this node is fine as is, status=disabled => please pay attention and make explicit decision to enable if it's safe, status=reserved => here be dragons
<javierm>
travmurav[m]: yeah
<javierm>
mripard: yup, agree
<javierm>
mripard, travmurav[m]: also, my understanding is that the pre-processing part is really something in dtc and not really part of the spec
<javierm>
so when having a plain dts with no included dtsi, it makes even more sense to have enabled as the default
<mripard>
javierm: the DT syntax has includes too
<mripard>
/include "path"/; iirc, or something like that
<javierm>
mripard: oh, indeed. There's a "6.1 Compiler directives" section so is part of the spec
<javierm>
I didn't know that
<mripard>
it's what we had at the very beginning before moving to the C preprocessor to handle includes
<travmurav[m]>
iirc the dtc include was pretty limited though so using cpp is much more convenient anwyay
<mripard>
iirc it was mostly to handle defines, but I can't remember why we switched to C-style includes
<mripard>
maybe to handle defines in the included DTSI?
<travmurav[m]>
dt binding includes with define only header files I'd assume
<travmurav[m]>
somehow I thought it doesn't handle extra include search paths but it's not the case, there is an arg for that
<javierm>
and also dt bindings includes that could use CPP macros ?
<javierm>
so I wasn't that wrong then, there's some compiler directives in the spec but the CPP support is an implementation detail in dtc ?
<travmurav[m]>
more like dtc doesn't even know it was fed cpp preprocessed code I'd say
<javierm>
travmurav[m]: got it. Is Kbuild then that pre-processes the dts(i) and passes the output dts to dtc
<travmurav[m]>
I have a project that builds (custom) dtb and we needed to switch to cpp at some point so what I did was running cpp to create -tmp.dts and then compile it with dtc, I assume Linux does something similar yeah
<javierm>
travmurav[m], mripard: cool, thanks for the explanation and reference. For some reason I misremembered that dtc was able to compile dts with included dtsi, but I see now that's not the case
<mripard>
it does, but only if /included
<javierm>
mripard: yeah, I meant with #include and CPP macros
<lumag>
being stuffed into 'don't register / set to on manually'
<marc|gonzalez>
lumag: for the HDMI redriver, I should document a single compat string in two binding docs, and leave the driver implementation up to a braver soul?
<marc|gonzalez>
(I meant the driver for "brainy" mode)
<marc|gonzalez>
Though I'm not sure how to specify the binding for the "brainy" mode, since we don't use that at all.
<lumag>
marc|gonzalez, just add a comment in the bindings that I2C mode is TODO
<rawoul>
lumag: in our case the redriver is in i2c mode, we just use the default chip config
<rawoul>
and Vcc/Vdd are always on
<lumag>
rawoul, oh, my. You shouldn't have said that :-D
<lumag>
Because now it becomes an i2c device,
<lumag>
While simple-bridge only handles the platform devices
<lumag>
I thought that you have I2C disabled.
<lumag>
So, I think, you'd still need an i2c driver for that, just note that advanced features are todo
<rawoul>
ok, that should be fine then. It wasn't really clear for me how to expose all the configurable knobs or test they actually work but if we don't have to do that it should be easy
<lumag>
well, you can't change your hardware. So you can implement whatever you can test and leave everything else as todo. Note, I haven't checked the actual programming, just skimmed through the datasheet and usage diagrams
<marc|gonzalez>
What does "platform" mean? That we can access the registers through MMIO?
<lumag>
More or less so. Either that or that it is not on some controllable bus like I2C
<lumag>
It was 'everything else' bucked, but auxiliary bus has blurred the boundaries a bit.
<lumag>
(at least from my pov)
<strongtz[m]>
narmstrong: `speaker-test -D plughw:0,1 -r 44100` also gets stuck for me
<strongtz[m]>
and unfortunately i2s pins are hard to access on my side :(
<strongtz[m]>
I wonder if you are getting any signal on the pins
<marc|gonzalez>
lumag: for HDMI phy regs, should I document everything I have, or only those I use?
<marc|gonzalez>
Meaning should I leave holes in the regmap, or document all
<lumag>
marc|gonzalez, up to you. remember that the split-away HDMI PHY will use plain header files for QSERDES and TX regions
<lumag>
OTOH if something was a part of msm-4.4, it's probably not worth not documenting it.
<lumag>
Especially for the PHY region
<marc|gonzalez>
So this time I make it an XML, and later will convert to plain text?
<lumag>
marc|gonzalez, unfortunately :-(
<marc|gonzalez>
No big deal, it's just a sed invocation away :)
<marc|gonzalez>
Actually, I have it as CPP defines, so I'll just keep that file around