On Wed, Apr 10, 2024 at 03:11:52PM +0100, Steven Price wrote:
> On 08/04/2024 08:35, Dan Carpenter wrote:
> > Hello Boris Brezillon,
> >
> > Commit de8548813824 ("drm/panthor: Add the scheduler logical block")
> > from Feb 29, 2024 (linux-next), leads to the following Smatch static
> > checker warning:
> >
> > drivers/gpu/drm/panthor/panthor_sched.c:1153
> > csg_slot_sync_state_locked()
> > error: uninitialized symbol 'new_state'.
> >
> > drivers/gpu/drm/panthor/panthor_sched.c
> > 1123 static void
> > 1124 csg_slot_sync_state_locked(struct panthor_device *ptdev, u32
> > csg_id)
> > 1125 {
> > 1126 struct panthor_csg_slot *csg_slot =
> > &ptdev->scheduler->csg_slots[csg_id];
> > 1127 struct panthor_fw_csg_iface *csg_iface;
> > 1128 struct panthor_group *group;
> > 1129 enum panthor_group_state new_state, old_state;
> > 1130
> > 1131 lockdep_assert_held(&ptdev->scheduler->lock);
> > 1132
> > 1133 csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> > 1134 group = csg_slot->group;
> > 1135
> > 1136 if (!group)
> > 1137 return;
> > 1138
> > 1139 old_state = group->state;
> > 1140 switch (csg_iface->output->ack & CSG_STATE_MASK) {
> > ^^^^^^^^^^^^^^
> > This mask is 0x7. Should it be 0x3? That would silence the static
> > checker warning.
>
> The mask is correct - it's effectively a hardware register (well it's
> read/written by the firmware on the hardware). States 4-7 are officially
> "reserved" and Should Not Happen™!
>
> I guess a "default:" case with a WARN_ON() would be the right solution.
>
> Steve
A WARN_ON() won't silence the warning. Plus WARN_ON() is not free in
terms of memory usage. And they're kind of controversial these days to
be honest.
One solution would be to just ignore the static checker warning. These
are a one time thing, and if people have questions in the future, they
can just search lore for this thread.
regards,
dan carpenter
