| *** tpb has joined #litex | 00:00 | |
| *** lf has quit IRC | 00:28 | |
| *** lf_ has joined #litex | 00:28 | |
| somlo | shorne: thanks, all taken care of; and no, I hadn't seen that particular post before :) | 00:31 |
|---|---|---|
| *** _whitelogger has quit IRC | 00:39 | |
| *** _whitelogger has joined #litex | 00:41 | |
| *** futarisIRCcloud has joined #litex | 02:19 | |
| *** _whitelogger has quit IRC | 02:57 | |
| *** _whitelogger has joined #litex | 02:59 | |
| *** _whitelogger has quit IRC | 03:54 | |
| *** _whitelogger has joined #litex | 03:56 | |
| *** Degi has quit IRC | 04:51 | |
| *** Degi has joined #litex | 04:53 | |
| *** Bertl_oO is now known as Bertl_zZ | 05:15 | |
| *** lambda has quit IRC | 07:40 | |
| *** lambda has joined #litex | 07:41 | |
| *** futarisIRCcloud has quit IRC | 07:49 | |
| *** lambda has quit IRC | 08:55 | |
| *** lambda has joined #litex | 08:58 | |
| *** _whitelogger has quit IRC | 09:27 | |
| *** _whitelogger has joined #litex | 09:29 | |
| *** leons has quit IRC | 10:40 | |
| *** leons has joined #litex | 10:53 | |
| *** lkcl has quit IRC | 11:32 | |
| *** lkcl has joined #litex | 11:45 | |
| *** futarisIRCcloud has joined #litex | 12:20 | |
| *** McJ19 has joined #litex | 13:08 | |
| *** McJ19 has quit IRC | 14:35 | |
| *** noziar has joined #litex | 14:53 | |
| *** noziar has quit IRC | 14:55 | |
| *** noziar has joined #litex | 15:02 | |
| *** noziar is now known as noziar2 | 15:03 | |
| *** noziar2 is now known as noziar | 15:03 | |
| *** noziar has quit IRC | 15:04 | |
| *** noziar has joined #litex | 15:05 | |
| *** noziar is now known as noziar2 | 15:06 | |
| *** noziar2 is now known as noziar | 15:06 | |
| *** martinraison has joined #litex | 15:07 | |
| *** noziar has quit IRC | 15:08 | |
| *** martinraison has quit IRC | 15:16 | |
| *** martinraison has joined #litex | 15:20 | |
| *** martinraison has quit IRC | 15:31 | |
| *** Bertl_zZ is now known as Bertl | 15:36 | |
| *** peeps[zen] has joined #litex | 17:19 | |
| *** peepsalot has quit IRC | 17:20 | |
| *** martinraison has joined #litex | 17:28 | |
| *** martinraison has quit IRC | 18:20 | |
| *** Dolu has joined #litex | 21:01 | |
| *** peeps[zen] is now known as peepsalot | 22:03 | |
| shorne | somlo: great, I for me the changes look good, I will wait to see if any other reviewers pick anything up | 22:15 |
| somlo | shorne: thanks! meanwhile, I have one more (small) change lined up for a v5: https://github.com/torvalds/linux/commit/da3fd1ca77e231b1f4d2b919b94e730d204ce51f#diff-e35c5e0b59bc8b9a4520b4b13c00cf8f4ff702786dc5cfc7a525806447867b10R78 | 22:29 |
| somlo | it's a small extra tightening of litex_set_reg() compared to what's in v4 right now | 22:30 |
| somlo | fewer variables, same effect | 22:30 |
| somlo | also, I've been thinking: I'm a bit uncomfortable exposing litex_[get|set]_reg() publicly as-is; would like to rename them to _litex_[get|set]_reg(), and use them internally from within litex_[read|write][8|16|32|64](); For "externally visible" purposes, I'd like the litex_[get|set]_reg() wrappers to check reg_size and throw some sort of error if it's outside the [1,8] interval (inclusive) | 22:32 |
| somlo | this latter thing would be a separate patch, of course | 22:33 |
| somlo | just looking for good examples on how to throw an error (which would have to be run-time, as call sites could pass in any valid "size_t" argument for reg_size...) | 22:34 |
| shorne | Also, some minor points, usually on each commit we add "Changes since vN:", in the dead space after "---" explained here https://kernelnewbies.org/PatchTipsAndTricks | 22:34 |
| tpb | Title: PatchTipsAndTricks - Linux Kernel Newbies (at kernelnewbies.org) | 22:34 |
| shorne | oh sorry, I didn't mean to post that | 22:34 |
| shorne | I am sending that on a mail now | 22:34 |
| somlo | which is also why I don't want to do the error check as part of litex_[read|write][8|16|32|64]() (where the reg_size is a compile-time constant, and guaranteed to be in the right interval) :) | 22:35 |
| somlo | ah, I've been doing that in the cover letter, but could certainly do it inside each commit as well | 22:35 |
| shorne | somlo: Its not a big thing for me, or since this series is not huge, but for others it may be hard to understand what is changing | 22:36 |
| somlo | makes sense | 22:36 |
| shorne | Maybe move litex_get|set_reg back into the .c file so its not exposed? I guess it could still be inline | 22:38 |
| somlo | They used to be explicitly exported via "EXPORT_SYMBOL_GPL()" for generic/arbitrary width registers | 22:39 |
| shorne | right | 22:40 |
| somlo | right now they're not used by any LiteX driver I know of (see https://github.com/torvalds/linux/compare/master...litex-hub:litex-rebase) -- everyone just uses litex_[read|write][8|16|32|64]() | 22:42 |
| somlo | so we can explicitly "unpublish" litex_[get|set]_reg() by prefixing them with a _ (iirc that's the convention for "don't call this directly as there's no promise it'll stay the same) | 22:43 |
| somlo | so that litex_[read|write][8|16|32|64]() can still use them | 22:44 |
| shorne | yeah, it makes sense | 22:44 |
| somlo | and for "public consumption" we either add wrappers with error checking (and those can go into the soc driver file or stay in the header, I don't care much), or we can *not* do that until it becomes necessary :) | 22:45 |
| shorne | I think usually the kernel uses 2 _, so it would be __litex_[get|set]_reg() | 22:46 |
| shorne | have a look around how they do it | 22:46 |
| somlo | actually I'm lying, some of the un-"curated" drivers that still only target 8-bit CSR data width do in fact use them directly, so adding wrappers is back on my todo list :) | 22:47 |
| somlo | right, the more underscores prefix a thing, the more "mere mortals" are discouraged from using it :) | 22:48 |
| somlo | I'll figure out the right amount of underscores for v5 as well | 22:48 |
| somlo | soon as I figure out how to deal with a failed error check on "reg_size" :) | 22:49 |
| somlo | I did check-patch and get_maintainer for v1; I'll run another checkpatch before v5 | 22:52 |
| somlo | I've been doing `git send-email -3 -vX --cover-letter --annotate` which is why adding change logs to individual patches wasn't on my radar... guess I'll get back to doing separate git-format-patch commands and tweaking the ascii patch files before sending the emails :) | 22:54 |
| somlo | shorne: I guess the canonical way to do error checking would be something like "if (reg_size < 1 || reg_size > 8) { pr_info(...); BUG(); }" | 23:47 |
| *** lkcl has quit IRC | 23:48 | |
| somlo | now trying to figure out the tradeoff between BUG() and WARN(), former feels more appropriate, since we're waaay off the reservation if the condition fails... | 23:48 |
| daveshah | BUG_ON might be even neater? | 23:52 |
| daveshah | Unless you really need the pr_info | 23:52 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!