marcan changed the topic of #asahi to: Asahi Linux: porting Linux to Apple Silicon macs | General project discussion | GitHub: https://alx.sh/g | Wiki: https://alx.sh/w | Topics: #asahi-dev #asahi-re #asahi-gpu #asahi-offtopic | Keep things on topic | Logs: https://alx.sh/l/asahi
acelogic2 has quit [Ping timeout: 272 seconds]
odmir has quit [Remote host closed the connection]
odmir has joined #asahi
<davidrysk[m]>
kettenis: are you saying that writing to E2H has an effect?
TheJollyRoger has quit [Remote host closed the connection]
TheJollyRoger has joined #asahi
<modwizcode>
cool, seems a lot nicer that way
<dhewg>
double "require" in 2nd patch description
<modwizcode>
what is "the second patch"
<dhewg>
oh those 7 :)
<dhewg>
*of
<modwizcode>
ah see it now
<arnd>
marcan: when you add the ioremap_np(), I'd suggest also adding a fallback implementation in asm-generic/io.h and a devm_ioremap_np() wrapper in include/linux/io.h+lib/devres.c
<arnd>
ah, I see you just split it up more fine-grained, and the generic fallback is already there
<marcan>
yeah, it's in pretty small pieces
<marcan>
devm_ioremap_np too
<modwizcode>
Isn't the ability to map without reserving kind of suspect? I feel like I remember reading about why that's not a thing
<marcan>
tons of drivers do it though, manually
<marcan>
e.g. the samsung one does it because it reserves/frees the region in other functions, though arguably it should be doing the mapping there too, but then the serial core is ancient and does not have the resource concept... so that would have to go in driver private stuff if we used that
<modwizcode>
The serial core is pre-reserving the memory (with the wrong attributes)?
<maz>
marcan: I think this looks reasonable. there will be some corner cases, I'm sure. but the mapping between DT and resources seem OK.
<marcan>
no, the serial core has callbacks to reserve/release the region, but the structures used just have a base/size, so it loses the attributes if any if we go through that
<arnd>
marcan: I would find it slightly easier to review if you combine the patches into slightly larger chunks, but that's not important. I assume that I'll just end up merging the lot through the soc tree in the end
<marcan>
I wasn't sure how much individual maintainers of those bits would want it split up, but I can certainly e.g. combine the generic asm ioremap_np(), IORESOURCE_MEM_NONPOSTED, devm_ioremap_np patches into one
<marcan>
(if you think that makes more sense)
<modwizcode>
I see, the serial core handles reservation seperately from mapping, but once it's reserved you can't use the mapping functions that reserve because it's already been reserved.
<arnd>
yes, I think that would help me.
<marcan>
ok, I'll do that in a bit (dinner first :))
Bublik has joined #asahi
<marcan>
modwizcode: I think the intent is the reserve/release functions should do the mapping too, but in the samsung driver the mapping is done ahead of time *before* reservation
<marcan>
however, the problem with moving mapping to the reserve/release functions is that by then the uart core thinks you just provided it with an mmio base/size to be mapped
<arnd>
I'm still trying to form an opinion on the of_mmio_is_nonposted() semantics. My earlier thoughts were that it should only have an effect on direct children of the bus, but it seems that robher has some other ideas (not sure if your implementation matches those)
<marcan>
I think I implemented what robher described
<marcan>
(as I understood it anyway)
<marcan>
i.e. walk up the tree until you find a positive/negative property, or until you hit a non-translatable boundary
<dhewg>
patch#1 vendor prefix needs a new desc, that still describes AAPL
<arnd>
I thought he only wanted to walk up the tree when there is an empty ranges property, but I may have misunderstood that
<marcan>
dhewg: yeah, I'll go through all the commit messages
<marcan>
that one's in my TODO
<marcan>
I just bulk s/AAPL/apple/ on everything
<dhewg>
alright
<arnd>
The only part I don't like in the series is the new devm_ioremap_resource_noreq() interface. I'd rather see the driver converted to either use devm_ioremap_resource(), or an open-coded devm_ioremap()
<marcan>
the problem with the latter is we lose the flags... so that needs to be checked separately
<arnd>
I need to check what it would take to use devm_ioremap_resource()
<arnd>
yes, I understand about the flag, it would then either have to call of_mmio_is_nonposted() or decide based on the compatible string
<arnd>
and both of these are slightly ugly as well, but are local to this driver
<marcan>
stm32-usart.c just uses dummies for request/release and devm_ioremap_resource
<marcan>
that approach would work
<marcan>
the whole serial API is... kind of antiquated anyway
<modwizcode>
classic work on a feature, you find the whole API is wrong :)
<marcan>
there's a reason for Documentation/driver-api/serial/tty.rst which starts like this:
<marcan>
=================
<marcan>
The Lockronomicon
<marcan>
=================
<marcan>
Your guide to the ancient and twisted locking policies of the tty layer and
<marcan>
the warped logic behind them. Beware all ye who read on.
<modwizcode>
yeah the TTY layer as a whole is... something else
<marcan>
arnd: honestly I think ditching _noreq and just doing what stm32-usart does is better
<modwizcode>
imo it mashes too many things together for what anyone actually needs it to do.
<modwizcode>
Definitely think ditching _noreq if you can is good
<marcan>
not just that one too
<marcan>
heck there are more drivers doing that than plain devm_ioremap
<marcan>
so I think it should be fine
<arnd>
marcan: I don't see the code that actually does the registration you mention in the comment, where is that?
<arnd>
s/registration/request_resource/
<marcan>
request_mem_region in s3c24xx_serial_request_port
<arnd>
ok, got it
<marcan>
arnd: I just changed it, ok to force push?
<arnd>
sure
<marcan>
done. tested working fine.
<arnd>
marcan: fairly sure you can just remove the two now-empty callbacks completely
<arnd>
otherwise looks good to me
<marcan>
ha, I was wondering why all the other drivers had dummies
<marcan>
a7cfaf165e changed it in 2016
<arnd>
yep, just found that commit as well
<j`ey>
marcan: yay more cleanups!
<marcan>
arnd: re-pushed
<marcan>
patches with more - than + are always the best kind
<j`ey>
youre not going to beat arnd though, since he's deleting all those old platforms :P
<dhewg>
"earlycon" is still in the dts
<arnd>
marcan: the ioremap stuff all looks good to me, just needs a DT binding description for the new properties, and a confirmation from robher that these are the semantics he wants
<marcan>
yup, I have the binding in my TODO
<arnd>
marcan: how is the mapping done for earlycon?
<marcan>
not yet :)
<marcan>
that needs a parallel implementation of the of stuff for FDTs
<arnd>
ok, then I can stop looking for that
<marcan>
and then just open-coding it in earlycon.c
<arnd>
I wonder if you can just hardcode it to ioremap_np() in apple_m1_early_console_setup()
<maz>
I'd be tempted to use nGnRnE for fixmap device mappings by default if I was sure it wouldn't introduce any regression...
<marcan>
yeah, I was thinking of that
<marcan>
bbiab, got some takeout pizza :)
<marcan>
maz: I think kettenis said they use nGnRnE for everything by default, which is some evidence that it isn't broken on other devices
<marcan>
I don't know if mapping the same region nGnRnE and nGnRE is legal though, per spec (which would happen for keep_bootcon on other platforms)
<robher>
marcan, arnd: looks okay to me in terms of semantics. The only thing is we walk up the whole tree for everyone else on every node. I think we should bail out on !apple. See some of the other quirks in that file which save quirk enable/disable in a static var.
<maz>
marcan: aliasing with different attributes can cause havoc in some interconnects.
<marcan>
thought so
<marcan>
robher: just as an optimization?
<robher>
marcan: yes
<modwizcode>
I think I read that you're not supposed to alias with different attributes. But I couldn't find anything for V8 in the ARM about it except that it had very specific definitions for the behavior.
<marcan>
robher: sure, I'll add something :)
maor26 has quit [Read error: Connection reset by peer]
<arnd>
modwizcode: maybe earlycon can be a simple hack like
<modwizcode>
Slightly offtopic: Where do I want to look if I want to change a kernel page I have to writable?
<arnd>
modwizcode: we had some discussions about conflicting page attributes in Linux years ago, those proably resulted in the current language in the reference manual. I much preferred the powerpc behavior of causing a checkstop to be honest
<modwizcode>
I was reading LWN articles about the issues I think
<modwizcode>
I feel like not allowing multiple mappings with different attributes causes a lot of pain
<arnd>
modwizcode: maybe look at aarch64_insn_patch_text()
<arnd>
modwizcode: do you want to have it permanently writable, or just write something to it and then return it to read-only?
<marcan>
arnd: I think that kind of thing would work. It's certainly simple.
<modwizcode>
the latter
<marcan>
I can just pull that in if you're OK with that
<arnd>
yes, just add "Suggested-by: Arnd Bergmann <arnd@arndb.de>"
<marcan>
will do
acelogic has joined #asahi
maor26 has joined #asahi
imobilis has quit [Quit: bye]
imobilis has joined #asahi
mikewilks[m] has quit [Quit: Idle for 30+ days]
nondi_ocesan[m] has joined #asahi
Tokamak has joined #asahi
danilonc has quit [Quit: WeeChat 1.9.1]
danilonc has joined #asahi
danilonc has joined #asahi
danilonc has quit [Changing host]
danilonc has quit [Client Quit]
danilonc has joined #asahi
danilonc has quit [Client Quit]
danilonc has joined #asahi
danilonc has quit [Client Quit]
danilonc has joined #asahi
danilonc has quit [Disconnected by services]
danilonc_ has joined #asahi
danilonc has joined #asahi
danilonc_ has quit [Client Quit]
awordnot has joined #asahi
kettenis has quit [Quit: leaving]
kettenis has joined #asahi
mndza has quit [Read error: Connection reset by peer]
mndza has joined #asahi
hir0 has quit [Quit: hir0]
VinDuv has joined #asahi
mps has joined #asahi
kido has quit [Changing host]
kido has joined #asahi
hir0 has joined #asahi
brandas has joined #asahi
brandas_ has quit [Ping timeout: 264 seconds]
roxfan2 is now known as roxfan
Bublik has quit [Ping timeout: 272 seconds]
Bublik has joined #asahi
raster has quit [Quit: Gettin' stinky!]
hir0 has quit [Quit: hir0]
leah2 has quit [Ping timeout: 260 seconds]
leah2 has joined #asahi
kettenis has quit [Quit: leaving]
brandas has quit [Quit: quit]
brandas has joined #asahi
raster has joined #asahi
mndza has quit [Ping timeout: 256 seconds]
raster has quit [Quit: Gettin' stinky!]
VinDuv has quit [Quit: Leaving.]
amw has joined #asahi
austriancoder has quit [Read error: Connection reset by peer]
austriancoder has joined #asahi
jamadazi has joined #asahi
kettenis has joined #asahi
linkmauve has quit [Ping timeout: 256 seconds]
jamadazi has quit [Read error: Connection reset by peer]
<kettenis>
just pushed changes to make the serial port work in u-boot