Messages in this thread Patch in this message | | | From | "Winkler, Tomas" <> | Subject | RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" | Date | Tue, 28 Jul 2020 22:14:06 +0000 |
| |
> -----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 { > >>>
| |