Hi Simon,
On 12. 12. 20 16:35, Simon Glass wrote:
> Hi Michal,
>
> On Thu, 3 Dec 2020 at 02:13, Michal Simek <michal.si...@xilinx.com> wrote:
>>
>> Some drivers like LCD connected via SPI requires explicit sync function
>> which copy framebuffer content over SPI to controller to display.
>> This hook doesn't exist yet that's why introduce it via video operations.
>>
>> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
>> ---
>>
>> Simon: Please review this. I didn't find existing way how this can be done
>> that's why I am introducing this hook. Also maybe name can be named a
>> little bit differently. That's why waiting for better suggestion.
>> ---
>> drivers/video/video-uclass.c | 5 +++++
>> include/video.h | 13 +++++++++++++
>> 2 files changed, 18 insertions(+)
>
>> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
>> index 650891e49dd0..ba52a6c7125b 100644
>> --- a/drivers/video/video-uclass.c
>> +++ b/drivers/video/video-uclass.c
>> @@ -174,6 +174,11 @@ void video_set_default_colors(struct udevice *dev, bool
>> invert)
>> /* Flush video activity to the caches */
>> void video_sync(struct udevice *vid, bool force)
>> {
>> + struct video_ops *ops = video_get_ops(vid);
>> +
>> + if (ops && ops->video_sync)
>> + (void)ops->video_sync(vid);
>
> We should update video_sync() to return an error.
I sent v2 with fixing this.
>
>> +
>> /*
>> * flush_dcache_range() is declared in common.h but it seems that
>> some
>> * architectures do not actually implement it. Is there a way to find
>> diff --git a/include/video.h b/include/video.h
>> index 9d09d2409af6..acac3f6b3c8d 100644
>> --- a/include/video.h
>> +++ b/include/video.h
>> @@ -115,7 +115,20 @@ struct video_priv {
>> };
>>
>> /* Placeholder - there are no video operations at present */
>
> Need to update comment
Removed.
>
>> +/**
>> + * struct video_ops - structure for keeping video operations
>> + */
>> struct video_ops {
>> + /**
>> + * video_sync - Synchronize FB with device
>> + *
>> + * Some device like SPI based LCD displays needs synchronization when
>> + * data in an FB is available. For these devices implement video_sync
>> + * hook to call a sync function
>> + *
>> + * @vid: Video device udevice structure
>
> @return
I have looked at kernel-doc format and completely redo this to pass
kernel-doc script.
There are a lot of issues reported by that script. Would be good to run
it by patman automatically to be more visible for everybody.
Thanks,
Michal
