Re: [PATCH v2 2/4] add stih-cec driver

[Benjamin Gaignard <benjamin.gaignard@linaro.org>]

Fixed and tested, thanks.

I will wait for any others comments before send a v3.

Regards,
Benjamin

2016-09-14 14:58 GMT+02:00 Hans Verkuil <hverkuil@xs4all.nl>:
> Hi Benjamin,
>
> Just one comment:
>
> On 09/14/2016 01:24 PM, Benjamin Gaignard wrote:
>> This patch implement CEC driver for stih4xx platform.
>> Driver compliance has been test with cec-ctl and
>> cec-compliance tools.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>>  drivers/staging/media/Kconfig           |   2 +
>>  drivers/staging/media/Makefile          |   1 +
>>  drivers/staging/media/st-cec/Kconfig    |   8 +
>>  drivers/staging/media/st-cec/Makefile   |   1 +
>>  drivers/staging/media/st-cec/stih-cec.c | 382 ++++++++++++++++++++++++++++++++
>>  5 files changed, 394 insertions(+)
>>  create mode 100644 drivers/staging/media/st-cec/Kconfig
>>  create mode 100644 drivers/staging/media/st-cec/Makefile
>>  create mode 100644 drivers/staging/media/st-cec/stih-cec.c
>>
>
> <snip>
>
>> +static void stih_rx_done(struct stih_cec *cec, u32 status)
>> +{
>> +     struct cec_msg *msg = &cec->rx_msg;
>
> You can just say:
>
>         struct cec_msg msg = {};
>
> and drop rx_msg.
>
>> +     u8 i;
>> +
>> +     if (status & CEC_RX_ERROR_MIN)
>> +             return;
>> +
>> +     if (status & CEC_RX_ERROR_MAX)
>> +             return;
>> +
>> +     memset(msg, 0x00, sizeof(*msg));
>> +     msg->len = readl(cec->regs + CEC_DATA_ARRAY_STATUS) & 0x1f;
>> +
>> +     if (!msg-len)
>> +             return;
>> +
>> +     if (msg->len > 16)
>> +             msg->len = 16;
>> +
>> +     for (i = 0; i < msg->len; i++)
>> +             msg->msg[i] = readl(cec->regs + CEC_RX_DATA_BASE + i);
>> +
>> +     cec_received_msg(cec->adap, msg);
>
> cec_received_msg will copy the contents, so it is OK if it is gone after
> this call.
>
>> +}
>
> Regards,
>
>         Hans



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog