Re: [PATCH v3 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig <alyssa@collabora.com>]

> 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