- 08 Jul, 2022 40 commits
-
-
Hans de Goede authored
When ia_css_rmgr_acq_vbuf() enters the code path where it uses the local "struct ia_css_rmgr_vbuf_handle v" on the stack it relies on v.count==0 so that ia_css_rmgr_refcount_retain_vbuf allocates a new handle. Explicitly set v.count to 0 rather then it being whatever was on the stack. Link: https://lore.kernel.org/linux-media/20220612160556.108264-3-hdegoede@redhat.comSigned-off-by:
Hans de Goede <hdegoede@redhat.com> Signed-off-by:
Mauro Carvalho Chehab <mchehab@kernel.org>
-
Hans de Goede authored
The gcc is warning about returning a pointer to a local variable is a false positive. The type of handle is "struct ia_css_rmgr_vbuf_handle **" and "h.vptr" is left to NULL, so the "if ((*handle)->vptr == 0x0)" check always succeeds when the "*handle = &h;" statement which gcc warns about executes. Leading to this statement being executed: rmgr_pop_handle(pool, handle); If that succeeds, then *handle has been set to point to one of the pre-allocated array of handles, so it no longer points to h. If that fails the following statement will be executed: /* Note that handle will change to an internally maintained one */ ia_css_rmgr_refcount_retain_vbuf(handle); Which allocated a new handle from the array of pre-allocated handles and then makes *handle point to this. So the address of h is actually never returned. The fix for the false-postive compiler warning actually breaks the code, the new: **handle = h; is part of a "if (pool->copy_on_write) { ... }" which means that the handle where *handle points to should be treated read-only, IOW **handle must never be set, instead *handle must be set to point to a new handle (with a copy of the contents of the old handle). The old code correctly did this and the new fixed code gets this wrong. Note there is another patch in this series, which fixes the warning in another way. Link: https://lore.kernel.org/linux-media/20220612160556.108264-2-hdegoede@redhat.com Fixes: fa145137 ("media: atomisp: don't pass a pointer to a local variable") Signed-off-by: -
Haowen Bai authored
The info->data is dereferencing before null checking, so move it after checking. Link: https://lore.kernel.org/linux-media/1653897481-25681-1-git-send-email-baihaowen@meizu.comSigned-off-by:
Haowen Bai <baihaowen@meizu.com> Reviewed-by:
Tommaso Merciai <tommaso.merciai@amarulasolutions.com> Signed-off-by:
-
Xiaomeng Tong authored
The three bugs are here: __func__, s3a_buf->s3a_data->exp_id); __func__, md_buf->metadata->exp_id); __func__, dis_buf->dis_data->exp_id); The list iterator 's3a_buf/md_buf/dis_buf' will point to a bogus position containing HEAD if the list is empty or no element is found. This case must be checked before any use of the iterator, otherwise it will lead to a invalid memory access. To fix this bug, add an check. Use a new variable '*_iter' as the list iterator, while use the old variable '*_buf' as a dedicated pointer to point to the found element. Link: https://lore.kernel.org/linux-media/20220414041415.3342-1-xiam0nd.tong@gmail.com Cc: stable@vger.kernel.org Fixes: ad85094b ("Revert "media: staging: atomisp: Remove driver"") Signed-off-by:
Xiaomeng Tong <xiam0nd.tong@gmail.com> Signed-off-by:
-
Fabio M. De Francesco authored
The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible. The same is true for kmap_atomic(). In file pci/hmm/hmm.c, function hmm_store() test if we are in atomic context and, if so, it calls kmap_atomic(), if not, it calls kmap(). First of all, in_atomic() shouldn't be used in drivers. This macro cannot always detect atomic context; in particular, it cannot know about held spinlocks in non-preemptible kernels. Notwithstanding what it is said above, this code doesn't need to care whether or not it is executing in atomic context. It can simply use kmap_local_page() / kunmap_local() that can instead do the mapping / unmapping regardless of the context. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Therefore, hmm_store()() is a function where the use of kmap_local_page() in place of both kmap() and kmap_atomic() is correctly suited. Convert the calls of kmap() / kunmap() and kmap_atomic() / kunmap_atomic() to kmap_local_page() / kunmap_local() and drop the unnecessary tests which test if the code is in atomic context. Link: https://lore.kernel.org/linux-media/20220413225531.9425-1-fmdefrancesco@gmail.comSigned-off-by:
Fabio M. De Francesco <fmdefrancesco@gmail.com> Reviewed-by:
Ira Weiny <ira.weiny@intel.com> Tested-by:
-
Fabio M. De Francesco authored
The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible. In file pci/hmm/hmm.c, function hmm_set() calls kmap() / kunmap() where kmap_local_page() can instead do the mapping. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Therefore, hmm_set()() is a function where the use of kmap_local_page() in place of kmap() is correctly suited. Convert the calls of kmap() / kunmap() to kmap_local_page() / kunmap_local(). Link: https://lore.kernel.org/linux-media/20220413212210.18494-1-fmdefrancesco@gmail.comSigned-off-by:
-
Fabio M. De Francesco authored
The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. load_and_flush_by_kmap() is a function where the use of kmap_local_page() in place of kmap() is correctly suited. Convert load_and_flush_by_kmap() from kmap() to kmap_local_page(). Link: https://lore.kernel.org/linux-media/20220408223129.3844-1-fmdefrancesco@gmail.comSigned-off-by:
-
Tom Rix authored
Clang static analysis reports this representative issue atomisp-ov2722.c:920:3: warning: 3rd function call argument is an uninitialized value dev_err(&client->dev, "sensor_id_high = 0x%x\n", high); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ high and low are only set when ov2722_read_reg() is successful. Reporting the high value when there is an error is not meaningful. The later read for low is not checked. high and low are or-ed together and checked against a non zero value. Remove the unneeded error reporting for high. Initialize high and low to 0 and use the id check to determine if the reads were successful The later read for revision is not checked. If it fails the old high value will be used and the revision will be misreported. Since the revision is only reported and not checked or stored it is not necessary to return if the read with successful. This makes the ret variable unnecessary so remove it. Link: https://lore.kernel.org/linux-media/20220326191853.2914552-1-trix@redhat.comSigned-off-by:
Tom Rix <trix@redhat.com> Signed-off-by:
-
Hans de Goede authored
Add a files documenting what I've learned about the driver while working on various cleanups. Link: https://lore.kernel.org/linux-media/20220615205037.16549-41-hdegoede@redhat.comReviewed-by:
Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by:
-
Hans de Goede authored
The force argument to the __destroy_pipe[s]() and __destroy_stream[s]() functions is always true. Remove the argument and remove the code necessary to handle the false case. Link: https://lore.kernel.org/linux-media/20220615205037.16549-40-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Use atomisp_destroy_pipes_stream_force() in 4 more places, instead of open coding it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-39-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Use atomisp_css_update_stream() in 2 more places, instead of open coding it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-38-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Use atomisp_create_pipes_stream() in 2 more places, instead of open coding it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-37-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
__destroy_streams() and __destroy_pipes() may return an error. Log a warning when either of them fails. Link: https://lore.kernel.org/linux-media/20220615205037.16549-36-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
The functions called by atomisp_create_pipes_stream() can fail, add error checking for them. Link: https://lore.kernel.org/linux-media/20220615205037.16549-35-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
On ia_css_pipe_get_info() errors, destroy both the streams as well as the pipes which were created. Link: https://lore.kernel.org/linux-media/20220615205037.16549-34-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
hmm_page_object only stores a struct page pointer, so we can just use the hmm_bo.pages page pointer array everywhere. Link: https://lore.kernel.org/linux-media/20220615205037.16549-33-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
HMM_BO_SHARE is not supported by the hmm_bo code at all, drop it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-32-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
highmem is always false, drop it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-31-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Make hmm_alloc() only take size as a parameter and remove other parameters. since all callers always pass the same flags. Link: https://lore.kernel.org/linux-media/20220615205037.16549-30-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Most hmm_alloc() callers want BO_PRIVATE type memory. Add a hmm_create_from_userdata() helper for other cases so that the hmm_alloc() calls for all the callers who don't want this can be simplied. Link: https://lore.kernel.org/linux-media/20220615205037.16549-29-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
hmm_isp_vaddr_to_host_vaddr() and hmm_host_vaddr_to_hrt_vaddr() are unused, remove them. Link: https://lore.kernel.org/linux-media/20220615205037.16549-28-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Drop the ATOMISP_MAP_FLAG_CACHED flag, it is never set anywhere; also drop the matching "cached" parameter to hmm[_bo]_alloc which value was derived form the never set flag. Drop the ATOMISP_MAP_FLAG_NOFLUSH, it is not used anywhere. Link: https://lore.kernel.org/linux-media/20220615205037.16549-27-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
This flag is only used by one hmm_alloc() caller, drop it and make the caller call hmm_set(ptr, 0, size) itself to do the clearing. Link: https://lore.kernel.org/linux-media/20220615205037.16549-26-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Drop the ATOMISP_ACC_FW_LOAD_* defines, these are no longer used anywhere. Link: https://lore.kernel.org/linux-media/20220615205037.16549-25-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
atomisp_is_acc_enabled() always returns false now, remove it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-24-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
atomisp_css_acc_done() is no longer used anywhere, remove it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-23-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
With the removal of the ACC ioctls and atomisp_acc.c asc.acc.pipeline never gets set, so it is always NULL. Remove asc.acc.pipeline and drop checks for it being NULL / !NULL. Link: https://lore.kernel.org/linux-media/20220615205037.16549-22-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
With the removal of the ACC ioctls and atomisp_acc.c a whole bunch of atomisp_*css_* functions is no longer used, remove them. Link: https://lore.kernel.org/linux-media/20220615205037.16549-21-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
With the ACC ioctls removed sd->acc.fw is always empty turning the atomisp_acc.c code into no-ops, remove it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-20-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
These ioctls allow userspace to load custom programs into the ISP, which: a) Seems dangerous b) Cannot be used by opensource userspace since there is no FOSS code to create such programs b) These seem to be unused even by the Android closed source camera code (they don't show up in a strace of the camera app) So removing these seems be a good idea. Another reason to remove these is that atomisp_acc_map() is the only user of the userptr functionality in hmm_alloc(), so it gets in the way of further cleanups / simplification of the hmm code. Link: https://lore.kernel.org/linux-media/20220615205037.16549-19-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
The comment documenting hmm_bo_allocated() was copied (and not modified) from the comment documenting hmm_bo_alloc(), so there are 2 copies of the hmm_bo_alloc() documentation. Remove the copy of the comment above the hmm_bo_allocated() prototype. Link: https://lore.kernel.org/linux-media/20220615205037.16549-18-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
hmm_bo_get_page_info() is not used anywhere, remove it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-17-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Since the hmm-pool code has been removed this now always gets set to HMM_PAGE_TYPE_GENERAL, so just remove it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-16-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
ia_css_frame_map() has only one caller which passes a hardcoded 0 for the attribute argument, drop it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-15-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Since we have removed the hmm pools these are completely meaningless now. Link: https://lore.kernel.org/linux-media/20220615205037.16549-14-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Without pool support the (optional) debug logging done by these is not really meaningful, drop it all. Link: https://lore.kernel.org/linux-media/20220615205037.16549-13-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
Since we never register any pools, this is all dead code, remove it. Link: https://lore.kernel.org/linux-media/20220615205037.16549-12-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
There are no callers of this code atm; and looking at the atomisp memory-management code if anything we want to make it simpler and not re-introduce use of these pools, so remove the pool code. Link: https://lore.kernel.org/linux-media/20220615205037.16549-11-hdegoede@redhat.comReviewed-by:
-
Hans de Goede authored
These are no-ops, so lets remove them. Link: https://lore.kernel.org/linux-media/20220615205037.16549-10-hdegoede@redhat.comSigned-off-by:
-
