lkml.org 
[lkml]   [2020]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member"
Date


> -----Original Message-----
> From: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Sent: Wednesday, July 29, 2020 01:15
> To: Winkler, Tomas <tomas.winkler@intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> kernel@vger.kernel.org; Gustavo A . R . Silva <gustavoars@kernel.org>
> Subject: Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array
> with flexible-array member"
>
>
>
> On 7/28/20 17:01, Winkler, Tomas wrote:
> >
> >
> >> -----Original Message-----
> >> From: Gustavo A. R. Silva <gustavo@embeddedor.com>
> >> Sent: Wednesday, July 29, 2020 00:57
> >> To: Winkler, Tomas <tomas.winkler@intel.com>; Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org>
> >> Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> >> kernel@vger.kernel.org; Gustavo A . R . Silva <gustavoars@kernel.org>
> >> Subject: Re: [char-misc-next] Revert "mei: hdcp: Replace one-element
> >> array with flexible-array member"
> >>
> >> Hi Tomas,
> >>
> >> On 7/28/20 16:41, Tomas Winkler wrote:
> >>> Greg please revert, this commit it changes size of struct
> >>> wired_cmd_repeater_auth_stream_req_in, this is not what firmware is
> >>> expecting.
> >>
> >> Could you elaborate on what's the firmware expecting, exactly?
> > struct wired_cmd_repeater_auth_stream_req_in {
> > - struct hdcp2_streamid_type streams[1];
> > + struct hdcp2_streamid_type streams[];
> > }
> >
> > But then you have, which you haven't changed to + 1 byte =
> > mei_cldev_send(cldev, (u8 *)&verify_mprime_in,
> > sizeof(verify_mprime_in));
> >
>
> I see, this is the kind of feedback I need from people that knows the code
> better. Thanks!
>
> > But that's not the major point. Point is that we should be able to
> > review and test the code before it is merged. You haven't run it, right?
> > There is MAINTAINERS file for a reason.
> >
>
> I'm using this command:
>
> $ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback
>
> and this is the output for both files:
>
> $ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback
> drivers/misc/mei/hdcp/mei_hdcp.c Arnd Bergmann <arnd@arndb.de>
> (supporter:CHAR and MISC DRIVERS) Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> (supporter:CHAR and MISC DRIVERS) linux-
> kernel@vger.kernel.org (open list) $ scripts/get_maintainer.pl --nokeywords --
> nogit --nogit-fallback drivers/misc/mei/hdcp/mei_hdcp.h Arnd Bergmann
> <arnd@arndb.de> (supporter:CHAR and MISC DRIVERS) Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> (supporter:CHAR and MISC DRIVERS) linux-
> kernel@vger.kernel.org (open list)


> For some reason you don't appear on the list above. Do you see anything
> wrong with the command I'm running to get the maintainers and lists?

Not sure, it should be caught by drivers/misc/mei/* ?
Maybe it is not recursive? Need to check the script,
frankly I usually check this manually.

INTEL MANAGEMENT ENGINE (mei)
M: Tomas Winkler <tomas.winkler@intel.com>
L: linux-kernel@vger.kernel.org
S: Supported
F: Documentation/driver-api/mei/*
F: drivers/misc/mei/*
F: drivers/watchdog/mei_wdt.c
F: include/linux/mei_cl_bus.h
F: include/uapi/linux/mei.h
F: samples/mei/*


>
> Thanks
> --
> Gustavo
>
> >>
> >>> I really do not appreciate that the code is bypassing driver
> >>> maintaner review, I think this is a minimum we can ask for, this is
> >>> not for a first time.
> >>>
> >>> This reverts commit c56967d674e361ebe716e66992e3c5332b25ac1f.
> >>>
> >>> Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
> >>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> >>> ---
> >>> drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
> >>> drivers/misc/mei/hdcp/mei_hdcp.h | 2 +-
> >>> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> b/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> index d1d3e025ca0e..e6c3dc595617 100644
> >>> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> >>> @@ -572,7 +572,7 @@ static int mei_hdcp_verify_mprime(struct device
> >> *dev,
> >>> HDCP_2_2_MPRIME_LEN);
> >>> drm_hdcp_cpu_to_be24(verify_mprime_in.seq_num_m, data-
> seq_num_m);
> >>> memcpy(verify_mprime_in.streams, data->streams,
> >>> - array_size(data->k, sizeof(*data->streams)));
> >>> + (data->k * sizeof(struct hdcp2_streamid_type)));
> >>>
> >>> verify_mprime_in.k = cpu_to_be16(data->k);
> >>>
> >>> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> b/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> index 834757f5e072..18ffc773fa18 100644
> >>> --- a/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> +++ b/drivers/misc/mei/hdcp/mei_hdcp.h
> >>> @@ -358,7 +358,7 @@ struct wired_cmd_repeater_auth_stream_req_in
> {
> >>> u8
> >> seq_num_m[HDCP_2_2_SEQ_NUM_LEN];
> >>> u8 m_prime[HDCP_2_2_MPRIME_LEN];
> >>> __be16 k;
> >>> - struct hdcp2_streamid_type streams[];
> >>> + struct hdcp2_streamid_type streams[1];
> >>> } __packed;
> >>>
> >>> struct wired_cmd_repeater_auth_stream_req_out {
> >>>
\
 
 \ /
  Last update: 2020-07-29 00:15    [W:0.087 / U:1.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site