On 2/22/19 12:17 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Thu, Feb 21, 2019 at 03:21:45PM +0100, Hans Verkuil wrote:
>> Ensure that this pointer is set to NULL after it is freed.
>> The vimc driver has a static media_entity and after
>> unbinding and rebinding the vimc device the media code will
>> try to free this pointer again since it wasn't set to NULL.
>
> As this fixes a problem in vimc, should you add a Fixes: tag ? To avoid
The Fixes tag is really for cases where a patch introduces a bug, whereas
this was always wrong.
> other similar problems, I think the vimc driver should allocate the
> media_device and other device data dynamically at probe time. Bundling
> them with the platform_device in struct vimc_device isn't a good idea.
It's not actually wrong as such, just unusual. Which actually makes it a
good test case.
Anyway, the upcoming "vimc: add configfs API to configure the topology" patch
makes this dynamic. Waiting for a v2 of that patch.
Regards,
Hans
>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>> ---
>> drivers/media/media-entity.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>> index 0b1cb3559140..7b2a2cc95530 100644
>> --- a/drivers/media/media-entity.c
>> +++ b/drivers/media/media-entity.c
>> @@ -88,6 +88,7 @@ EXPORT_SYMBOL_GPL(__media_entity_enum_init);
>> void media_entity_enum_cleanup(struct media_entity_enum *ent_enum)
>> {
>> kfree(ent_enum->bmap);
>> + ent_enum->bmap = NULL;
>> }
>> EXPORT_SYMBOL_GPL(media_entity_enum_cleanup);
>>
>
