> Sorry about this blunder. > > >> + slot = panfrost_job_get_slot(job); > >> + slot = slot ? slot : 0; > > > >`slot = slot ? slot : 0` is a no-op. Delete the line. > > I think what I meant here was 'slot = (slot >= 0) ? slot : 0;' but for some > reason I blundered again. The point of this was ensuring the slot value wouldn't > end up wrapping about the maximum unsigned integer value when using it as an > array offset, in the off-chance that panfrost_job_get_slot() ever returned a > negative value. > > In v4 I've instead rewritten this as a sanity check: > > WARN_ON(slot < 0); No, this doesn't make sense. There at most 3 job slots -- 0, 1, and 2. > Although perhaps in the future panfrost_job_get_slot should return an unsigned > integer instead? Sure. Kernel style doesn't seem big on unsigned, if this were mesa it would return unsigned. Returning u8 or u32 seems reasonable at any rate. > >As a general note, I'd appreciate breaking out the panfrost_regs.h > >changes into a separate patch, as they are a logically separate clean > >up to make room for this patch. Thanks. > > Done in v4. Thanks! Alyssa
