<narmstrong>
rockosov: it doesn't block other review, they are independent bindings/drivers, drop the a1 clk bindings includes from the other bindings and use fake labels instead (&clkc CLKID_XXX ==> &clkc_xxxx), it will drop the unecessary dependency
<rockosov>
narmstrong: sorry, I meant from functional point of view. Also we're preparing uboot clk/pinctrl/other drivers and it would be nice to have aligned CLK bindings between kernel and uboot, so we are waiting final bindings on the kernel side to apply it at the uboot side.
jacobk has quit [Ping timeout: 260 seconds]
marc|gonzalez has joined #linux-amlogic
<marc|gonzalez>
narmstrong: xdarklight: if the DT patch and driver patch are not applied atomically, then the intermediate step is not bisectable ?
<narmstrong>
marc|gonzalez: it's more a maintainance problem, ultimately both changes should be in the same commit, but since it's a low priority driver and we are still in -rc, it's ok to apply both changes separately
<marc|gonzalez>
It's a low priority driver, but it clashes with the canvas, which IIUC if broken breaks a whole lotta things?
<marc|gonzalez>
can you say a few words about the maintenance issue? It's because various parts have different maintainers?
<marc|gonzalez>
Is it handled differently for high priority drivers?
<narmstrong>
yep different maintainers, and it means it's ok to have a temporary break until the final release
<narmstrong>
if you push the new patchset, I'll apply the 2 DT fixes and send them asap, then we'll need to wait for the driver fix to be applied
<marc|gonzalez>
If it's OK to break next, I should tell maintainers to bugger off when they tell me to test on next
<narmstrong>
yep it's ok to break next when it's not important stuff, here we'll break something already broken
<marc|gonzalez>
OK, I see the logic.
<marc|gonzalez>
narmstrong: I do a lot of things manually, so my setup must be different from yours who use it frequently. Can you explain why adding the patches to an existing thread messes things up?
<narmstrong>
marc|gonzalez: for people with mail clients presenting email threads, it will mix the old and nee patches in the same thread which is hard to figure out, and the "b4" tool will mix the threads and be very confused
<narmstrong>
honestly, you should start using the `b4 prep` feature, it does all the manual steps automatically, and preserves the cover-letter, creates tags, determines links to older versions, extracts maintainers & list, ...
<narmstrong>
updates trailers, ...
<rockosov>
narmstrong: I have one question regarding pwm a1/s4 drivers. Some days ago Chrisitan Hewitt sent patchset for pwm s4 support and it's part of a1 pwm support which we are preparing for posting now. What should we do in this situation? Christian patchset is under review and not stable, ours as well. I see one option, but I don't know it's right way or not. I think we need to send our patch
<rockosov>
without dependency to Christian patchset and merge it in the future when you ack both of them. What do you think about it?
<narmstrong>
rockosov: do you have a really different solution for a1 ?
<rockosov>
narmstrong: Ah, yes, I mean this one, from Heiner. Don't know why I've decided it's from Christian, somehow I've mixed it with another review from linux-amlogic in my mind, sorry.
<rockosov>
narmstrong: We have ext_clk in out implementation as well, but also we have constant and polarity bits implementation plus we've prepared a fix with more accurate meson_pwm_cnt_to_ns() calculation.
<narmstrong>
rockosov: usually we don't duplicate changes on the the list, so the right way to handle this would be to rebase on heiner's patch, and send your remaining changes by specifying you depend on heiner's patch
<rockosov>
narmstrong: Okay, get it. We will apply Heiner patch and be based on it.
George41 has joined #linux-amlogic
<narmstrong>
rockosov: don't hesitate reviewing Heiner's changed and propose some changes
<rockosov>
narmstrong: Sure, we will provide feedback to Heiner patchset if any. Thank you for clarification. Also one small question regarding secure pwrc fix which we disccused at the Friday (https://lore.kernel.org/all/20230324145557.27797-1-ddrokosov@sberdevices.ru/). It was acked by Martin. Does it mean there no any action from my side required, right?
<narmstrong>
rockosov: I'll apply it
<rockosov>
narmstrong: Good news, thank you!
Lila-Kuh has joined #linux-amlogic
rockosov has quit [Ping timeout: 265 seconds]
rockosov has joined #linux-amlogic
f_ has joined #linux-amlogic
exkc has joined #linux-amlogic
<marc|gonzalez>
How do I get git send-email to lookup the list of recipients?
<marc|gonzalez>
Come on, I'm not gonna learn a whole new tool for a trivial patch set... I'm 5 minutes away from doing this manually again
<narmstrong>
the link you shared looks interesting, honestly I was doing this by hand before using b4...
<narmstrong>
but instead of adding those to send-email I'll add them to format-patch instead
<narmstrong>
so you can fixup the To/Cc before sending the patches
f_ has quit [Ping timeout: 276 seconds]
<rockosov>
jbrunet: If you don't mind, let's discuss public/private clock bindings approach here. I suppose it may be faster and after the final decision I can make the final implementation :)
<rockosov>
First of all, let's summarize what we have on the table.
<rockosov>
First approach: "public/private clock bindings based on splitting bindings headers"
<rockosov>
Prons:
<rockosov>
1. we have only one compile-time CLKID bindings linear list, splitted into several ranges between two headers.
<rockosov>
2. if we want to move clock object from public to private and vice versa, it will not be a big deal.
<rockosov>
Cons:
<rockosov>
1. Device tree developer can use clock object from private list with 'raw' binding, so we don't protect internal clock objects at all.
<rockosov>
2. DT bindings maitainer doesn't like such approach (I mean Krzysztof)
<rockosov>
Second approach: "public/private clock objects based on limited clock hw provider registration"
<rockosov>
Prons:
<rockosov>
1. Device tree developer doesn't have ability to point private clock object, because it's not exported to clk hw provider.
<rockosov>
1. It's not a big problem to move private clock object to public list, but it doesn't work vice versa, because it breaks bindings ABI.
<rockosov>
2. It's new approach for meson clock drivers.
<rockosov>
From my point of view, both approaches are good to use. Both of them have prons and cons, but solve the main question.
<rockosov>
Only one thing which I don't understand and really sorry for that. It's legal question.
<rockosov>
You as Amlogic maitainer support the first option, Krzysztof as bindings maitainer likes the second one.
<rockosov>
For me it looks like a dead lock, sorry :-)
<rockosov>
I really appreciate if you can explain the right way here.
<narmstrong>
Third approach, publish all clocks IDs in the bindings
<rockosov>
narmstrong: :)
<rockosov>
narmstrong: I suppose we don't want to open some internal clocks (like dividers)
<narmstrong>
I don't see any reasons, except no wanting users to mess with those clocks, but it's already possible with the current model because the IDs a valid from the ABI PoV
<marc|gonzalez>
narmstrong: errf, I think I used the wrong patch prefix for 2/3 ?
<rockosov>
narmstrong: Hmmm, there is a grain of truth. All clocks are public, most of them support auto-reparenting. If we want to setup the parent directly in the future, we just need to fix driver side. ABI will not be broken.
<rockosov>
jbrunet: What do you think about Neil's approach (all clocks are public in the include/dt-bindings)?
<jbrunet>
1st and 3rd approach is the same as far as I am concerned. 1st does give but does not actually restrict anything as Neil is pointing out.
<jbrunet>
Krzysztof expressed concerned in a fairly "automated" way, even though the problem of non continuous IDs had already been discussed and resolved.
<rockosov>
jbrunet: In the 1st approach we have one trap. Let's imagine some dt developer uses 'raw' dt binding from our hidden CLKID list. After some time we change private CLKID numbers, because it's private for us, really? Such situaion will break ABI.
<jbrunet>
Never said you were allowed to change the meaning of the ID, you are just allowed to have them declared in the bindings (or not)
<rockosov>
jbrunet: Or we must follow the strict rule: "There is no option to chage public or private CLKID list numbers"
<jbrunet>
It's been like that for the past 5 years or so ... we manage to avoid the "trap" so far
<rockosov>
jbrunet: Okay, got it. So I'll rework patch series to the first approach then. Thank you for clarification!
exkc has quit [Ping timeout: 255 seconds]
naoki has quit [Quit: naoki]
exkc has joined #linux-amlogic
jacobk has joined #linux-amlogic
exkc has quit [Ping timeout: 250 seconds]
Lila-Kuh has quit [Remote host closed the connection]
Lila-Kuh has joined #linux-amlogic
f_ has joined #linux-amlogic
f_[xmpp] has joined #linux-amlogic
f_[xmpp] has quit [Changing host]
f_ has joined #linux-amlogic
f_ has quit [Changing host]
<marc|gonzalez>
narmstrong: in the end, it looks like the perf driver panics my setup :(
<narmstrong>
marc|gonzalez: anyway it's unrelated to the fixes you posted
<narmstrong>
and the crash occurs in brcmf_sdiod_skbuff_read()
<marc|gonzalez>
Crashes are all over the place.
<marc|gonzalez>
I will check whether I have the fix you mentioned
<marc|gonzalez>
No, I don't have that patch, I'm on v6.2
<marc|gonzalez>
The driver author was active only a month ago & now is AWOL?
exkc has joined #linux-amlogic
<marc|gonzalez>
narmstrong: apparently there are different revisions of G12A, my two boards report Revision 28:b & Revision 28:e -- I assume Amlogic fixed "stuff" in the later revs?
exkc has quit [Remote host closed the connection]
<narmstrong>
I’ll try to do a test run tomorrow
jacobk has quit [Ping timeout: 276 seconds]
<marc|gonzalez>
I can confirm that "amlogic,dram-access-quirk" is required for "Revision 28:b" :)
<marc|gonzalez>
I can't get WiFi working at all on the 28:e board
<rockosov>
narmstrong: thank you for the information, I have been using lore.kernel.org to follow kernel amlogic topics all this time, but don't know about groups.io for uboot.
<rockosov>
Guys, does anyone know official uboot IRC channel link? Could you please share?
<rockosov>
Already found on the libera. Sorry for the noise.