<dhruvag2000>
2. #include "arch/arm64/boot/dts/ti/k3-pinctrl.h" in pinctrl-single.c
<dhruvag2000>
1. have #defines AM62X_WK_EN and AM62X_WK_EVT in arch/arm64/boot/dts/ti/k3-pinctrl.h
<dhruvag2000>
3. Then from my earlier patch the .irq_enable_mask = (1 << 29) would ust change to .irq_enable_mask = AM62X_WK_EN,
<dhruvag2000>
And the rest stays the same, that we chain the pmx as a wake irq and also have the compatible "am62-pinctrl" in it's DT node and that's how it will set the wk_en bit for supported devices by making use of the wake IRQ framework?
<dhruvag2000>
We have followed this pattern of having device specific wk_en and evt bits so far for omap 3,4,5,dra, and am43 right? Why not continue the same trend?
<NishanthMenon>
"2. #include "arch/arm64/boot/dts/ti/k3-pinctrl.h" in pinctrl-single.c" -> NAK
<NishanthMenon>
dhruvag2000: my comment was for you to download the trm for all k3 devices and look if the bits were common. if they are, then the driver is the place to do things like what was done.
<dhruvag2000>
I did take a brief look, seems like some J7 devices, AM62A do have commonality however AM64 for example had those bits marked reserved... Now I am not sure if reserved means 1 or all 0s but yeah.. AM64 stood out
<dhruvag2000>
> "2. #include "arch/arm64/boot/dts/ti/k3-pinctrl.h" in pinctrl-single.c" -> NAK
<dhruvag2000>
Sorry then could you elaborate on what you meant by "Why cant we set this in the k3-pinctrl.h and set it once?" exactly? @tmlind_ then also said to "define the bit offsets k3-pinctrl.h" , how exactly do I use these defined bit offsets from k3-pinctrl in the driver then?
<NishanthMenon>
dhruvag2000: please do a thorough read through of the trms - if all the SoCs where relevant have the same bits, then compatible should be able to pick it up. as was discussed in the chain, it does'nt make sense for the dts to enable the bits all the time since the driver will toggle on need. the SoCs where the bit is set to reserved obviously dont have the functionality and compatible should'nt be used.
<dhruvag2000>
okay @NishanthMenon , I will go through the K3 TRM's wherever daisychain is supported and ensure that the wk_en and evt bits are at the same positions. If yes, then this patch is good to go?
<NishanthMenon>
i leave the "good to go" to Tony :)
<NishanthMenon>
commit message should address the review comments though - it was'nt clear, hence the discussion occurred.
<NishanthMenon>
and the compatible should be the oldest device which supported iodaisy chain bits..
jluthra has quit [Quit: WeeChat 4.0.1]
jluthra has joined #linux-ti
florian_kc has joined #linux-ti
<dhruvag2000>
got it, that will remain am625.
<dhruvag2000>
Thanks for clarifying!
<dhruvag2000>
Will add in commit message that all K3 SOCs will need these bits to use io daisychain and the same compatible can be used on supported platforms.
<tmlind_>
dhruvag2000: for including the soc specific pinctrl header.. if there are soc specific bit defines with the same name it won't work
<tmlind_>
if the define names conflict i mean
<tmlind_>
nice pinctrl-single yaml binding finally merged by linus w :)
<NishanthMenon>
yep +1
darkapex has joined #linux-ti
darkapex has quit [Quit: WeeChat 2.3]
darkapex has joined #linux-ti
<dhruvag2000>
Awesome! Let me rebase on latest -next then. (Hope it builds)
<dhruvag2000>
Ah it's not in next yet, I guess it will appear tomorrow?
<NishanthMenon>
yes
ikarso has quit [Quit: Connection closed for inactivity]
<NishanthMenon>
pivi: ^^ is on linux-next next-20230807
florian has quit [Ping timeout: 246 seconds]
<NishanthMenon>
aaah cruft.. I think the main_conf is still not converted to j721e-system-controller thingy
florian has joined #linux-ti
<NishanthMenon>
pivi: sorry, i think this is going to have to wait yaml fixups - it looks like a contrived mess atm
<NishanthMenon>
jluthra: ^^ I have responded on the list - please address
<pivi>
NishanthMenon: I think that the specific node I am adding is not generating any warning on the am62, I understand the issue on the am62a, I understand the issue with the planned conversion of syscon, but my specific patch for am62 would not change a line with/without the change on the syscon node and does not introduce any new issue.
<pivi>
NishanthMenon: why not just removing the am62a change and get it in? any cleanup on the syscon node would be unrelated to my change. the specific nodes I am adding are all fine and would stay fine.
<NishanthMenon>
pivi: sure that is an option. I just don't like catching make dtbs_check at the last min.
<pivi>
NishanthMenon: I did check the bindings that verdin-am62 was fine, and given that it was also the am62-main dtsi was. I did not though about am62a
<NishanthMenon>
:(
<pivi>
NishanthMenon: I can remove the am62a, if it works for you. Or we'll have to wait for jluthra or someone else to cleanup this syscon node
<pivi>
NishanthMenon: anyway, I assumed the commit picked from git.ti.com was fine, I checked it was fine on the verdin-am62, I did not consider am62a
<pivi>
my fault on that
<NishanthMenon>
jluthra: are you ok if we drop am62a?
<pivi>
NishanthMenon: let me do it in the meantime
<NishanthMenon>
syscon cleanup: I already have ranted internally ;) hopefully folks will listen
<pivi>
... updated v3 runnin in our internal CI .... (yes, it runs the DT checker ... but only for our boards ;-)
<pivi>
ok, so I checked verdins (as usuale per our CI), and in addition ti/k3-am62a7-sk.dtb and ti/k3-am625-sk.dtb. No new warnings.
<pivi>
I'll send a v3 and you'll decide what's best