<Mis012[m]>
Forty-Bot: well, imho if all it does is that it serves to locate the core_clk, that can be done with a single core_clk *
<Mis012[m]>
not sure what Linux uses it for
<Mis012[m]>
if anything, the fat pointer is dynamically allocated while a normal pointer is just a number
<Mis012[m]>
and we already have a pointer to the fat pointer anyway, since it's malloced
<Forty-Bot>
right now there is no subsystem-managed per-clock memory
<Forty-Bot>
everything is done either by the clock driver or the requesting driver
<Forty-Bot>
idk why Linux has it but you have to have a separate struct clk for every consumer
<Forty-Bot>
so maybe it's a devlink thing
<Forty-Bot>
sorry, device link
<Forty-Bot>
somehow those are two separate things...
<Mis012[m]>
well, my idea is, why not have the of_xlate function return a pointer to core_clk
<Mis012[m]>
instead of filling in information that serves to get the core_clk later
<Mis012[m]>
it should already exist by the time of_xlate gets executed
redbrain has quit [Ping timeout: 255 seconds]
<Mis012[m]>
u-boot doesn't need to store anything per consumer afaict
<Mis012[m]>
not sure if there is maybe some binding along the lines of `<&clk_provider CLK1 per_consumer_thing>` or at least a possibility of such binding being accepted by the devicetree maintainers
<Forty-Bot>
so I think we have this design because a lot of cpus have hundreds of clocks, and U-boot may only need to menipulate a dozen or two
<Forty-Bot>
so it doesn't make sense to allocate a bunch of memory up front for clocks we don't know if we're going to use
<Forty-Bot>
that said, clearly CCF driver authors think it's easier to copy from linux
<Forty-Bot>
but that doesn't change that for the majority of clock drivers, we don't really know what clock we have until xlate
<Mis012[m]>
Forty-Bot: well, the memory is allocated anyway
<Forty-Bot>
not always
<Forty-Bot>
I mean, sometimes it's just an entry in a table
<Forty-Bot>
a clock can take up a byte or two in a good design
<Mis012[m]>
the clocks are stored as a linked list so that compiled out clocks don't take up space as would be the case with an array
<Forty-Bot>
you don't need linked lists for that
<Mis012[m]>
Linux uses id-indexed arrays
<Forty-Bot>
actually, IMO linked lists are a bit wasteful in your case
<Forty-Bot>
*our
<Forty-Bot>
although it really only becomes a problem when you need to set a clock in SPL
<Mis012[m]>
well, linked lists are what imx driver uses instead of an id-indexed array with a bunch of empty space due to having to comply with the ids in the bindings
<Forty-Bot>
yeah, but imx also uses CCF so
<Mis012[m]>
I want to use CCF obviously
<Forty-Bot>
I know, but I'm still not a fan of its design
<Mis012[m]>
which is what we're brainstorming how to fix
<Mis012[m]>
I have 1MB just of L2 TCM, I don't think I'll run out of SRAM
<Forty-Bot>
it's beyond saving by design
<Forty-Bot>
the things I don't like are the features :)
<Mis012[m]>
well, the stated goal is to reduce footprint
<Mis012[m]>
so if I don't make it larger it's fair game
<Forty-Bot>
I have no objections
<Forty-Bot>
but it also has to keep the rest of the clock subsystem reasonaby sized
<Mis012[m]>
Forty-Bot: so what I wonder is, would we ever need the per-consumer fat pointer struct
<Mis012[m]>
currently it could be replaced by a pointer to clk_core saving space afaict
<Forty-Bot>
well, we need the clock driver pointer, the various ids, and possibly the clock ops
<Mis012[m]>
the id and ops belong in core_clk
<Forty-Bot>
IMO there's not much advantage to using regular pointers
<Forty-Bot>
I don't see what putting them in some clk_core buys us
prabhakarlad has quit [Quit: Client closed]
<Mis012[m]>
clk_core is needed
<Mis012[m]>
the fat pointer isn't
<Forty-Bot>
and why is that
<Mis012[m]>
information about the clock needs to be stored somewhere, either in an id-indexed array like Linux does or in a non-id-indexed array but with core_clk having an id so we can loop over the array's members
redbrain has joined #u-boot
<Forty-Bot>
id indexing can't be used by the clock subsystem, since some drivers have sparse ids or ids which span multiple numbers
<Mis012[m]>
the id-indexed array will probably have a lot of empty space, yes
<Mis012[m]>
imx driver uses linked lists, but non-id-indexed array would presumably work too
<Mis012[m]>
this is what I currently have in u-boot
<Mis012[m]>
additionally should have the .id
vagrantc has quit [Quit: leaving]
<Mis012[m]>
the parent stuff could be put in a wrapper struct and the ops would know to use to_clk_with_parents if the clock has parents
<Mis012[m]>
bonus points if we can make that transparent to the driver
<Mis012[m]>
which seems doable
<Mis012[m]>
arguably most clocks probably have parents, but if the imx people disagree... would have to see when I do the size comparison before and after
<Forty-Bot>
all clocks have parents, but some parents are things like data streams
<Forty-Bot>
and some clocks don't have paren't because they're crystals
<Mis012[m]>
well, sure, but how many crystals do you have
<Forty-Bot>
up to 10?
<Mis012[m]>
right, then wrapper struct it is
<Forty-Bot>
hm?
ikarso has joined #u-boot
<Mis012[m]>
well, I guess that doesn't really work
ikarso has quit [Quit: Connection closed for inactivity]
naoki has joined #u-boot
naoki has quit [Client Quit]
runcom has joined #u-boot
d-s-e has quit [Ping timeout: 248 seconds]
mmu_man has quit [Ping timeout: 256 seconds]
runcom has quit [Ping timeout: 252 seconds]
GNUtoo has joined #u-boot
runcom has joined #u-boot
<Mis012[m]>
yeah... I don't actually see Linux using the clk struct for anything that would be relevant to u-boot
<Mis012[m]>
but I'm certainly not all that unhappy about this blatant waste of RAM on the imx person's part because it makes it easier for me to make it both saner and smaller :P
runcom has quit [Ping timeout: 252 seconds]
matthias_bgg has quit [Quit: Leaving]
mmu_man has joined #u-boot
mmu_man has quit [Ping timeout: 240 seconds]
d-s-e has joined #u-boot
guillaume_g has quit [Quit: Konversation terminated!]
monstr has quit [Remote host closed the connection]
vagrantc has joined #u-boot
ullbeking has quit [Quit: Connection closed for inactivity]