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
hspak has quit [Ping timeout: 252 seconds]
jeffmiw_ has quit [Remote host closed the connection]
troseman has joined #asahi
zkrx has quit [Ping timeout: 240 seconds]
odmir has quit [Remote host closed the connection]
odmir has joined #asahi
hspak has joined #asahi
odmir has quit [Ping timeout: 268 seconds]
zkrx has joined #asahi
odmir has joined #asahi
odmir has quit [Remote host closed the connection]
odmir has joined #asahi
bdju has quit [Read error: Connection reset by peer]
bdju has joined #asahi
phiologe has quit [Ping timeout: 250 seconds]
phiologe has joined #asahi
marvin24 has quit [Ping timeout: 245 seconds]
marvin24 has joined #asahi
troseman has quit [Ping timeout: 260 seconds]
TheJollyRoger has quit [Remote host closed the connection]
TheJollyRoger has joined #asahi
odmir has quit [Remote host closed the connection]
odmir has joined #asahi
odmir has quit [Ping timeout: 240 seconds]
jeffmiw_ has joined #asahi
jeffmiw_ has quit [Ping timeout: 240 seconds]
tomtastic has quit [Ping timeout: 240 seconds]
tomtastic has joined #asahi
VinDuv has joined #asahi
mxw39 has quit [Quit: Konversation terminated!]
ephe_meral1 has joined #asahi
h_ro[m] has quit [Quit: Idle for 30+ days]
angelXwind has quit [Ping timeout: 240 seconds]
jeffmiw_ has joined #asahi
luca020400 has quit [Quit: WeeChat 3.1]
luca020400 has joined #asahi
akemin_dayo has joined #asahi
jeffmiw_ has quit []
Necrosporus has quit [Ping timeout: 240 seconds]
vimal has quit [Quit: Leaving]
vimal has joined #asahi
Necrosporus has joined #asahi
vimal has quit [Remote host closed the connection]
<marcan>
sven: isn't this ioremapping a whole page for every 32-bit clock index? this seems sub-optimal
<marcan>
shouldn't it be one clock controller node with clock IDs corresponding to register indices? or perhaps one clock controller with subclock nodes that have reg offsets relative to the parent?
<sven>
yeah, i thought about that
<sven>
for the first one i'd then have to hardcode child/parent relationships in the driver itself
<sven>
and i haven't found any other driver doing the second thing
<marcan>
is any other soc doing what you do here?
<marcan>
sven: look at assigned-clock-parents and assigned-clocks
<marcan>
I think that can specify parent relationships within a single clock controller
<sven>
there's a general gpio gate clock driver but that's based on the gpio subssytem. the closest one is renesas,r7s72100-mstp-clocks i think
<marcan>
sven: look at freescale/imx8mq.dtsi
<marcan>
on clk:
<marcan>
that sets up parents within a single clock controller
<marcan>
as well as some default rates
<marcan>
you should be able to do the same thing, and just have one single clock controller instance
<sven>
oh, i thought assigned-clocks was just for the consumers
<marcan>
looks like it works everywhere
<marcan>
which makes sense if you think about it
<marcan>
:)
<sven>
right.. so essentially pretend that the controller is a consumer of itself and do it there
<marcan>
yup
<marcan>
which it *is*
<sven>
makes sense
<sven>
yeah
<sven>
good thing i kept my original code where it was just a single controller ;)
<marcan>
hahaha
<sven>
well, two actually because they seem to have two. but same thing
<marcan>
sure
klaus__ has joined #asahi
klaus___ has joined #asahi
klaus__ has quit [Quit: Lost terminal]
<kettenis_>
assigned-clock-parents is to initially set the muxes when there are multiple parent clocks to choose from
<kettenis_>
the benfit of the "one node per clock" strategy is that you don't have to define clock IDs and can make the clock hierarchy explicit in the device tree instead of coding it in the clock driver
<kettenis_>
but the "one node per clock module" model seems to be preferred by the Linux maintainers these days
zopieux has quit [Quit: zopieux]
<kettenis_>
btw, just pushed a draft pull request for the PCIe initialization code for m1n1
<kettenis_>
in the "one node per clock" model you can probably avoid ioremapping the same page multiple times by using a regmap
zopieux has quit [Ping timeout: 258 seconds]
zopieux has joined #asahi
<sven>
yeah, assigned-clock-parents won't work
<sven>
if i want a single clock controller node i think i either have to hardcode the parents in the driver or invent some new dt property
glibc has left #asahi [#asahi]
taziden has quit [Ping timeout: 246 seconds]
<marcan>
sven: what kettenis_ said, use a regmap and child nodes
<marcan>
you still want one device bound to the overall clock controller, but you can have subnodes for the actual clocks
<marcan>
option #2 above
<sven>
sure, it's just that no other soc seems to do that afaict
<marcan>
maybe ask robher? :)
<sven>
they seem to all use clock-indices together with hardcoded parents in the driver
<sven>
heh, now that would be too easy :-P
<marcan>
heh, ugly
<marcan>
but we're doing things right here (see: how I fixed the arm timer to suck less after several SoCs were already using dirty hacks to work around it)
<marcan>
so if we have to define a new binding or use a new style, we do :p
<kettenis_>
sunxi went from a node-per-clock to a single clock node a few years ago
<kettenis_>
I never understood why
<kettenis_>
anyway, to create a regmap you needs a "syscon" compatible node
ephe_meral1 has quit [Ping timeout: 250 seconds]
<kettenis_>
if you want the clock nodes to be children of that, it also needs a "simple-mfd" compatible
<marcan>
since the regmap is on real mmio, apparently another way to do it is as a bus
<marcan>
see: arch/arm/boot/dts/omap5-l4.dtsi
<marcan>
and omap54xx-clocks.dtsi
<kettenis_>
don't think that is a sensible example to follow
<kettenis_>
my understanding omap is done that way since there actually is a bus that can be powered on and off
taziden has joined #asahi
vimal has joined #asahi
odmir has joined #asahi
odmir has quit [Ping timeout: 240 seconds]
tmlind has joined #asahi
<tmlind>
marcan: the "new" clkctrl clocks for omaps use a clockdomain controller + reg offset + gate/mux/divider bit offset, no need to look at any of the old omap54xx-clocks.dtsi stuff for most part
<tmlind>
kettenis: there's a ti specific wrapper ip for each device target on the interconnect that needs to be configured for pm, well the hierarchy also brings genpd stuff for free then
<tmlind>
but yeah i'd recommend one dts node for a clock controller, then have the binding map the clock instances from that base with offset and optional bits if needed
<tmlind>
then it's worth checking if each clock controller instance might have some clock domain specific bit for pm for example?
<tmlind>
in any case, don't use any artificial indexing with dts for the instances, makes things hard if you find an extra clock somewhere in between later on :)
<marcan>
tmlind: the question is how to declare the hierarchy in the dts
<marcan>
subnodes of the clock controller? something else?
odmir_ has joined #asahi
<tmlind>
hmm so which hieararchy are you talking about? should not be needed for clocks?
<tmlind>
to me it sounded like you just want #clock-cells = <1>, or <2> if you also need to map bits in a single clock register
raster has quit [Quit: Gettin' stinky!]
<tmlind>
and assigned-clock-parents works just fine for a single node for a clock controller no problem
<sven>
to enable access to e.g. the uart mmio region we need to ungate a clock called uart0. but that clock itself is a child of uart_p which is a child of sio which is a child of sio_busif in apple's device tree. if any of the parents are disabled you can't enable the uart0 clock
<sven>
and the question is how and where to encode that toplogy. afaict most drivers just hardcode it in some .c file
choozy has joined #asahi
<tmlind>
yeah so those are on some device that's always powered, probably best to keep it out of the hierarchy, or probe those parts of the hierarchy first
<sven>
no, they are not on any other device afaict.
<tmlind>
well on some interconnect inside the soc i mean :)
<sven>
oh, sure
<sven>
but how do we represent that?
<tmlind>
well you could just have the clock controller as a top level node, along what's done for gic etc
<tmlind>
well aic in this case, not gic :)
<sven>
i'm not sure i understand
plainbits has joined #asahi
<sven>
let me look at some gic bindings though :)
<tmlind>
hmm so is the clock gate register stuffed somewhre within the uart address space? or is the uart clock gate in some clock controller device with a bunch of clocks?
<sven>
the second one
<sven>
it's in some clock controller device with a bunch of clocks
<marcan>
tmlind: it's not about the clock controller itself, it's about encoding the hierarchy of clocks it controls
<marcan>
i.e. dependencies
<tmlind>
yeah ok.. so is the clock controller really a child of the uart or am i missing something?
<sven>
the clock controller is separate. but the clocks which are children of that controller have a hierarchy
<sven>
and the uart needs one of these clocks
<tmlind>
ok makes more sense now thanks :)
<sven>
so the uart node just gets a clocks = <&clk-controller 25>; or something like that. but that clock with the id 25 needs clock 13 which in turn needs clock 7
<tmlind>
well you can do the clock tree hierarchy in the clock controller driver, no need to stuff it into dts
<marcan>
but then that makes the driver less generic
<marcan>
AIUI this clock stuff is uniform enough it ought to work for quite a few apple SoCs
<marcan>
but obviously they will have different hierarchies
<marcan>
if we start hardcoding the tree in the driver, that means the driver becomes soc-specific
<tmlind>
yeah that's where all the drivers are heading though to avoid custom bindings with the logic being the driver needs to understand the hardware :)
<tmlind>
sure if you can make something generic with minimal bindings, even better
<kettenis_>
I don't see the logic there though ;)
<tmlind>
so are you thinking to just process the apple dtb for clocks, then have driver use that?
<tmlind>
after translating to the standard dts format i mean
<marcan>
we're not processing the apple dtb, we're writing our own devicetrees
<marcan>
but I would much rather have drivers generic enough that they can work with multiple SoC generations if the register interface and such is the same, so we only need new devicetrees where possible
<tmlind>
sure that would be ideal if you can do that with mostly standard bindings :)
<marcan>
the plan is to write some tooling to parse and convert apple devicetrees as a human-guided operation for bringing up new boards and SoCs, but it wouldn't be structured as a straight conversion
<marcan>
more like a pile of python scripts that can, say, automatically fill in IRQ numbers and clocks and such, but without restricting how our bindings look
<tmlind>
yeah sure. so you're thinking the whole clock hierarchy is there too already in the apple dtb?
<sven>
it is
<tmlind>
ok, it's been few months since i looked that that stuff
<sven>
their nodes have a "clock-gates" property which indexes into a binary array that's passes as "devices" of the "arm-io/pmgr" node
<sven>
and that "devices" property contains the whole hierarchy
<tmlind>
heh
<sven>
it's a little weird :)
solarkraft has quit [Ping timeout: 240 seconds]
<marcan>
apple really likes to put blobs in DT properties
<marcan>
with random packed structures
<tmlind>
well you could use that to automatically generate the clock controller driver clocks and just expose the top level dts node :)
<marcan>
use that from where?
<tmlind>
parse from the apple dtb if it has the necessary data
<marcan>
and do what with the data? :)
<marcan>
linux doesn't touch the adt
<tmlind>
set up the clock controller driver clock instances for regs and bits
<marcan>
m1n1 is the bridge between adt and fdt, linux doesn't get to see the adt (by design)
<tmlind>
heh i mean one-time parse with some script to produce the driver internal data
<tmlind>
not parsed in the kernel
<marcan>
ah, and then embed it in the driver?
<marcan>
but then we're back to the driver being soc-specific
<tmlind>
yeah that's where linux has been heading good or bad.. see wc -l for drivers/clk and drivers/pinctrl for example
<tmlind>
well after find for .c files that is
<marcan>
I mean... I don't particularly care about "where linux has been heading" (especially when it's vendor people writing the code) and more about "where linux should head" :-)
<marcan>
if there's a good reason for doing it this way, fine
<marcan>
but I'd like to hear the reason :)
<tmlind>
well one issue is dealing with the sprawling amount of custom bindings, so the binding should be ideally generic, like interrupts-extended for example
<tmlind>
then some folks do not like the idea of a driver doing something purely based on devicetree data
<kettenis_>
I suppose the reasoning is that since the device tree is bundled with the kernel it doens't matter whether you add the data in the device tree or in the driver
<kettenis_>
but that doesn't apply here
<marcan>
well, ultimately custom bindings for any given block are pretty much necessary depending on what needs to be expressed, but at least here we were trying to look at ways of expressing this using established patterns
<marcan>
like regmap stuff and such
<tmlind>
yeah try to leave out custom bindings, that will save many merge cycles on mailing list discussions :)
<marcan>
I mean, DTs may live in the kernel tree but they aren't bundled inside the kernel
<marcan>
and doing this in the DT might mean the difference between a random Ubuntu release booting on newer Macs with just a m1n1 update, and needing to wait a year for a kernel bump
<tmlind>
personally, i do not like the ongoing static data bloat in drivers/clk and drivers/pinctrl, and ideally we'd have devicetree provide the necessary data for the booted processor
<tmlind>
sounds like expressing a clock hierarchy in a dts file in a generic way could be few years of mailing list discussions though :)
<kettenis_>
the generic clock bindings already provide a way to express the hierarchy
<tmlind>
oh, do you have some example to look at?
<kettenis_>
the only thing that's needed is a compatible for the apple-specific clock layout
<kettenis_>
register layout I mean
<tmlind>
hmm so what is the meaning of the apple-specific clock layout then?
<kettenis_>
you still need a small driver to actually set the right bits in the 32-bit clock register
<tmlind>
the clk24 there or something else?
<kettenis_>
compatible = "apple,pmgr-clk-gate";
<kettenis_>
the nodes that have that
<tmlind>
yeah ok like pcie_ref_clk
<kettenis_>
this is how Corellium did things in their "proof-of-concept"
<tmlind>
typing from it :)
<kettenis_>
as far as we can tell apple has been using the same 32-bit register layout for several enerations of iphones now
<kettenis_>
generatons even
<kettenis_>
i can't type, but you get the point
<tmlind>
so with the apple-specific clock layout, what's the info, like parent and rate?
<kettenis_>
the only thing that seems to vary is the number of registers (i.e. the number of clocks) and the hierarchy
<kettenis_>
so we think it is reasonable to assume this is likelyto stay the same in future SoCs
<sven>
these clocks are only used to enable access to mmio regions essentially. there are additional clocks that actually have a rate associated with them, which we mostly ignore for now
<sven>
("clock-ids" in the ADT which maps to clock-frequencies in the arm-io node)
<marcan>
they're clock gates
<sven>
(i think these "clock-ids" clocks are actually mux clocks pre-configured by iboot. but that's an educated guess)
<marcan>
keep in mind there's a power manager core too, I don't know how much of this is real registers and how much is somehow backed by that thing
<tmlind>
hmm right
<kettenis_>
you communicate with the "SMC" through a mailbox interface
<sven>
there are also many more regs in the pmgr node. who knows what's hidden there as well
<tmlind>
so is there any grouping of these clocks by the controller in the apple dtb?
vimal has quit [Remote host closed the connection]
<kettenis_>
not sure what you mean by this, but the 32-bit clock gate registers are part of a few (two?) larger blocks of mmio space
<sven>
yeah, they are part of two larger blocks
<sven>
there's also some "power domain" stuff but i haven't looked into that yet
<kettenis_>
the way corellium implements this you end up mapping a full page for each 32-bit register
<marcan>
that's what sven did in the patch above
<kettenis_>
hence the suggestion to use a regmap
<marcan>
which I complained about
<marcan>
:)
vimal has joined #asahi
<tmlind>
so there might be something like a clock controller device, then a bunch of gate or mux registers
<sven>
the mux registers are in a different mmio block fwiw
odmir_ has quit [Remote host closed the connection]
vimal has quit [Remote host closed the connection]