* [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" @ 2020-07-28 21:41 Tomas Winkler 2020-07-28 21:56 ` Gustavo A. R. Silva 0 siblings, 1 reply; 13+ messages in thread From: Tomas Winkler @ 2020-07-28 21:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Alexander Usyskin, linux-kernel, Tomas Winkler, Gustavo A . R . Silva Greg please revert, this commit it changes size of struct wired_cmd_repeater_auth_stream_req_in, this is not what firmware is expecting. 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 { -- 2.25.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 21:41 [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" Tomas Winkler @ 2020-07-28 21:56 ` Gustavo A. R. Silva 2020-07-28 22:01 ` Winkler, Tomas 0 siblings, 1 reply; 13+ messages in thread From: Gustavo A. R. Silva @ 2020-07-28 21:56 UTC (permalink / raw) To: Tomas Winkler, Greg Kroah-Hartman Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva 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? 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 { > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 21:56 ` Gustavo A. R. Silva @ 2020-07-28 22:01 ` Winkler, Tomas 2020-07-28 22:14 ` Gustavo A. R. Silva 0 siblings, 1 reply; 13+ messages in thread From: Winkler, Tomas @ 2020-07-28 22:01 UTC (permalink / raw) To: Gustavo A. R. Silva, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva > -----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)); 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. > > 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 { > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 22:01 ` Winkler, Tomas @ 2020-07-28 22:14 ` Gustavo A. R. Silva 2020-07-28 22:14 ` Winkler, Tomas 2020-07-28 22:17 ` Gustavo A. R. Silva 0 siblings, 2 replies; 13+ messages in thread From: Gustavo A. R. Silva @ 2020-07-28 22:14 UTC (permalink / raw) To: Winkler, Tomas, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva 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? 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 { >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 22:14 ` Gustavo A. R. Silva @ 2020-07-28 22:14 ` Winkler, Tomas 2020-07-28 22:29 ` Winkler, Tomas 2020-07-28 23:06 ` Joe Perches 2020-07-28 22:17 ` Gustavo A. R. Silva 1 sibling, 2 replies; 13+ messages in thread From: Winkler, Tomas @ 2020-07-28 22:14 UTC (permalink / raw) To: Gustavo A. R. Silva, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva > -----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 { > >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 22:14 ` Winkler, Tomas @ 2020-07-28 22:29 ` Winkler, Tomas 2020-07-28 23:04 ` Gustavo A. R. Silva 2020-07-28 23:06 ` Joe Perches 1 sibling, 1 reply; 13+ messages in thread From: Winkler, Tomas @ 2020-07-28 22:29 UTC (permalink / raw) To: Gustavo A. R. Silva, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva > > >> > > >> 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/* > It's not recursive, when I add drivers/misc/mei/hdcp/* . It works correctly I will post a patch. Thanks Tomas > > > > > 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 { > > >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 22:29 ` Winkler, Tomas @ 2020-07-28 23:04 ` Gustavo A. R. Silva 2020-07-28 23:07 ` Winkler, Tomas 2020-07-28 23:16 ` Gustavo A. R. Silva 0 siblings, 2 replies; 13+ messages in thread From: Gustavo A. R. Silva @ 2020-07-28 23:04 UTC (permalink / raw) To: Winkler, Tomas, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva Tomas, Please, see some comments below... On 7/28/20 17:29, Winkler, Tomas wrote: >>>>> >>>>> 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 don't think the fix for this is to add 1 byte, if any, it seems to be sizeof(*data->streams), or sizeof(struct hdcp2_streamid_type) what needs to be added. But it might be better to code something like this, instead: diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c index e6c3dc595617..7fe63c915548 100644 --- a/drivers/misc/mei/hdcp/mei_hdcp.c +++ b/drivers/misc/mei/hdcp/mei_hdcp.c @@ -572,12 +572,12 @@ 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, - (data->k * sizeof(struct hdcp2_streamid_type))); + (data->k * sizeof(*data->streams))); verify_mprime_in.k = cpu_to_be16(data->k); byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, - sizeof(verify_mprime_in)); + struct_size(&verify_mprime_in, streams, data->k)); if (byte < 0) { dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); return byte; struct_size(&verify_mprime_in, streams, data->k) will give us the size, in bytes, of struct wired_cmd_repeater_auth_stream_req_in plus the size in bytes for the streams[] flexible-array, which is determined by struct hdcp2_streamid_type and data->k. What do you think? See more comments below... >>> >>> 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/* >> > > It's not recursive, when I add drivers/misc/mei/hdcp/* . It works correctly > I will post a patch. > Great. I'm glad we got to the root cause of this issue. :) Thanks -- Gustavo > Thanks > Tomas > > >> >>> >>> 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 { >>>>>> ^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 23:04 ` Gustavo A. R. Silva @ 2020-07-28 23:07 ` Winkler, Tomas 2020-07-28 23:16 ` Gustavo A. R. Silva 1 sibling, 0 replies; 13+ messages in thread From: Winkler, Tomas @ 2020-07-28 23:07 UTC (permalink / raw) To: Gustavo A. R. Silva, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva > > Tomas, > > Please, see some comments below... > > On 7/28/20 17:29, Winkler, Tomas wrote: > >>>>> > >>>>> 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 don't think the fix for this is to add 1 byte, if any, it seems to be sizeof(*data- > >streams), or sizeof(struct hdcp2_streamid_type) what needs to be added. > > But it might be better to code something like this, instead: > > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c > b/drivers/misc/mei/hdcp/mei_hdcp.c > index e6c3dc595617..7fe63c915548 100644 > --- a/drivers/misc/mei/hdcp/mei_hdcp.c > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c > @@ -572,12 +572,12 @@ 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, > - (data->k * sizeof(struct hdcp2_streamid_type))); > + (data->k * sizeof(*data->streams))); > > verify_mprime_in.k = cpu_to_be16(data->k); > > byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, > - sizeof(verify_mprime_in)); > + struct_size(&verify_mprime_in, streams, > + data->k)); > if (byte < 0) { > dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); > return byte; > > struct_size(&verify_mprime_in, streams, data->k) will give us the size, in > bytes, of struct wired_cmd_repeater_auth_stream_req_in plus the size in > bytes for the streams[] flexible-array, which is determined by struct > hdcp2_streamid_type and > data->k. > > What do you think? I think I will talk to the firmware guys tomorrow morning... > > See more comments below... > > >>> > >>> 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/* > >> > > > > It's not recursive, when I add drivers/misc/mei/hdcp/* . It works > > correctly I will post a patch. > > > > Great. I'm glad we got to the root cause of this issue. :) > > Thanks > -- > Gustavo > > > Thanks > > Tomas > > > > > >> > >>> > >>> 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 { > >>>>>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 23:04 ` Gustavo A. R. Silva 2020-07-28 23:07 ` Winkler, Tomas @ 2020-07-28 23:16 ` Gustavo A. R. Silva 2020-07-29 3:57 ` Gustavo A. R. Silva 1 sibling, 1 reply; 13+ messages in thread From: Gustavo A. R. Silva @ 2020-07-28 23:16 UTC (permalink / raw) To: Winkler, Tomas, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva On 7/28/20 18:04, Gustavo A. R. Silva wrote: > Tomas, > > Please, see some comments below... > > On 7/28/20 17:29, Winkler, Tomas wrote: >>>>>> >>>>>> 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 don't think the fix for this is to add 1 byte, if any, it seems > to be sizeof(*data->streams), or sizeof(struct hdcp2_streamid_type) > what needs to be added. > > But it might be better to code something like this, instead: > > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c > index e6c3dc595617..7fe63c915548 100644 > --- a/drivers/misc/mei/hdcp/mei_hdcp.c > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c > @@ -572,12 +572,12 @@ 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, > - (data->k * sizeof(struct hdcp2_streamid_type))); > + (data->k * sizeof(*data->streams))); > > verify_mprime_in.k = cpu_to_be16(data->k); > > byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, > - sizeof(verify_mprime_in)); > + struct_size(&verify_mprime_in, streams, data->k)); > if (byte < 0) { > dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); > return byte; > > struct_size(&verify_mprime_in, streams, data->k) will give us the size, in bytes, > of struct wired_cmd_repeater_auth_stream_req_in plus the size in bytes for > the streams[] flexible-array, which is determined by struct hdcp2_streamid_type and > data->k. > > What do you think? > This is even better: diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c index e6c3dc595617..852374565cba 100644 --- a/drivers/misc/mei/hdcp/mei_hdcp.c +++ b/drivers/misc/mei/hdcp/mei_hdcp.c @@ -572,12 +572,12 @@ 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, - (data->k * sizeof(struct hdcp2_streamid_type))); + flex_array_size(&verify_mprime_in, streams, data->k)); verify_mprime_in.k = cpu_to_be16(data->k); byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, - sizeof(verify_mprime_in)); + struct_size(&verify_mprime_in, streams, data->k)); if (byte < 0) { dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); return byte; the flex_array_size() is a new helper that was designed for this situations. :) It calculates the size of a flexible array member within an enclosing structure, which is exactly the case. Thanks -- Gustavo > See more comments below... > >>>> >>>> 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/* >>> >> >> It's not recursive, when I add drivers/misc/mei/hdcp/* . It works correctly >> I will post a patch. >> > > Great. I'm glad we got to the root cause of this issue. :) > > Thanks > -- > Gustavo > >> Thanks >> Tomas >> >> >>> >>>> >>>> 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 { >>>>>>> ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 23:16 ` Gustavo A. R. Silva @ 2020-07-29 3:57 ` Gustavo A. R. Silva 0 siblings, 0 replies; 13+ messages in thread From: Gustavo A. R. Silva @ 2020-07-29 3:57 UTC (permalink / raw) To: Winkler, Tomas, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva On 7/28/20 18:16, Gustavo A. R. Silva wrote: > > > On 7/28/20 18:04, Gustavo A. R. Silva wrote: >> Tomas, >> >> Please, see some comments below... >> >> On 7/28/20 17:29, Winkler, Tomas wrote: >>>>>>> >>>>>>> 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. >>>>>>> Instead of reverting that commit, I think this patch should be applied to fix it, if Tomas consider this is the appropiate solution: diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c index d1d3e025ca0e..852374565cba 100644 --- a/drivers/misc/mei/hdcp/mei_hdcp.c +++ b/drivers/misc/mei/hdcp/mei_hdcp.c @@ -572,12 +572,12 @@ 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))); + flex_array_size(&verify_mprime_in, streams, data->k)); verify_mprime_in.k = cpu_to_be16(data->k); byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, - sizeof(verify_mprime_in)); + struct_size(&verify_mprime_in, streams, data->k)); if (byte < 0) { dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); return byte; I'll write up a proper patch after getting some feedback. Thanks -- Gustavo >>>>>>> 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 don't think the fix for this is to add 1 byte, if any, it seems >> to be sizeof(*data->streams), or sizeof(struct hdcp2_streamid_type) >> what needs to be added. >> >> But it might be better to code something like this, instead: >> >> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c >> index e6c3dc595617..7fe63c915548 100644 >> --- a/drivers/misc/mei/hdcp/mei_hdcp.c >> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c >> @@ -572,12 +572,12 @@ 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, >> - (data->k * sizeof(struct hdcp2_streamid_type))); >> + (data->k * sizeof(*data->streams))); >> >> verify_mprime_in.k = cpu_to_be16(data->k); >> >> byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, >> - sizeof(verify_mprime_in)); >> + struct_size(&verify_mprime_in, streams, data->k)); >> if (byte < 0) { >> dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); >> return byte; >> >> struct_size(&verify_mprime_in, streams, data->k) will give us the size, in bytes, >> of struct wired_cmd_repeater_auth_stream_req_in plus the size in bytes for >> the streams[] flexible-array, which is determined by struct hdcp2_streamid_type and >> data->k. >> >> What do you think? >> > > This is even better: > > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c > index e6c3dc595617..852374565cba 100644 > --- a/drivers/misc/mei/hdcp/mei_hdcp.c > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c > @@ -572,12 +572,12 @@ 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, > - (data->k * sizeof(struct hdcp2_streamid_type))); > + flex_array_size(&verify_mprime_in, streams, data->k)); > > verify_mprime_in.k = cpu_to_be16(data->k); > > byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, > - sizeof(verify_mprime_in)); > + struct_size(&verify_mprime_in, streams, data->k)); > if (byte < 0) { > dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte); > return byte; > > the flex_array_size() is a new helper that was designed for this situations. :) > It calculates the size of a flexible array member within an enclosing structure, > which is exactly the case. > > Thanks > -- > Gustavo > >> See more comments below... >> >>>>> >>>>> 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/* >>>> >>> >>> It's not recursive, when I add drivers/misc/mei/hdcp/* . It works correctly >>> I will post a patch. >>> >> >> Great. I'm glad we got to the root cause of this issue. :) >> >> Thanks >> -- >> Gustavo >> >>> Thanks >>> Tomas >>> >>> >>>> >>>>> >>>>> 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 { >>>>>>>> ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 22:14 ` Winkler, Tomas 2020-07-28 22:29 ` Winkler, Tomas @ 2020-07-28 23:06 ` Joe Perches 2020-07-28 23:08 ` Winkler, Tomas 1 sibling, 1 reply; 13+ messages in thread From: Joe Perches @ 2020-07-28 23:06 UTC (permalink / raw) To: Winkler, Tomas, Gustavo A. R. Silva, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva On Tue, 2020-07-28 at 22:14 +0000, Winkler, Tomas wrote: > > -----Original Message----- > > From: Gustavo A. R. Silva <gustavo@embeddedor.com> [] > > 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/* ? No, this pattern matches _only_ files in that particular directory and not any files in any subdirectory. > Maybe it is not recursive? It depends on the pattern. Patterns with trailing slashes match subdirectories too. from MAINTAINERS: F: *Files* and directories wildcard patterns. A trailing slash includes all files and subdirectory files. F: drivers/net/ all files in and below drivers/net F: drivers/net/* all files in drivers/net, but not below F: */net/* all files in "any top level directory"/net One pattern per line. Multiple F: lines acceptable. > Need to check the script, > frankly I usually check this manually. prob > > 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/* You probably want: 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 { > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 23:06 ` Joe Perches @ 2020-07-28 23:08 ` Winkler, Tomas 0 siblings, 0 replies; 13+ messages in thread From: Winkler, Tomas @ 2020-07-28 23:08 UTC (permalink / raw) To: Joe Perches, Gustavo A. R. Silva, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva > On Tue, 2020-07-28 at 22:14 +0000, Winkler, Tomas wrote: > > > -----Original Message----- > > > From: Gustavo A. R. Silva <gustavo@embeddedor.com> > [] > > > 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/* ? > > No, this pattern matches _only_ files in that particular directory and not any > files in any subdirectory. > > > Maybe it is not recursive? > > It depends on the pattern. > Patterns with trailing slashes match subdirectories too. > > from MAINTAINERS: > > F: *Files* and directories wildcard patterns. > A trailing slash includes all files and subdirectory files. > F: drivers/net/ all files in and below drivers/net > F: drivers/net/* all files in drivers/net, but not below > F: */net/* all files in "any top level directory"/net > One pattern per line. Multiple F: lines acceptable. > > > > Need to check the script, > > frankly I usually check this manually. prob > > > > 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/* > > You probably want: > > F: drivers/misc/mei/ Yes, thanks. Tomas ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" 2020-07-28 22:14 ` Gustavo A. R. Silva 2020-07-28 22:14 ` Winkler, Tomas @ 2020-07-28 22:17 ` Gustavo A. R. Silva 1 sibling, 0 replies; 13+ messages in thread From: Gustavo A. R. Silva @ 2020-07-28 22:17 UTC (permalink / raw) To: Winkler, Tomas, Greg Kroah-Hartman Cc: Usyskin, Alexander, linux-kernel@vger.kernel.org, Gustavo A . R . Silva On 7/28/20 17:14, Gustavo A. R. Silva wrote: > > > 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?> Notice that if I run this command: $ scripts/get_maintainer.pl -f 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) Ramalingam C <ramalingam.c@intel.com> (commit_signer:2/2=100%,authored:2/2=100%,added_lines:4/4=100%,removed_lines:13/13=100%) Tomas Winkler <tomas.winkler@intel.com> (commit_signer:2/2=100%) Uma Shankar <uma.shankar@intel.com> (commit_signer:2/2=100%) Jani Nikula <jani.nikula@intel.com> (commit_signer:2/2=100%) Shashank Sharma <shashank.sharma@intel.com> (commit_signer:2/2=100%) linux-kernel@vger.kernel.org (open list) You appear this time, but not as a maintainer. 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 { >>>> ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-07-29 3:51 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-28 21:41 [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member" Tomas Winkler 2020-07-28 21:56 ` Gustavo A. R. Silva 2020-07-28 22:01 ` Winkler, Tomas 2020-07-28 22:14 ` Gustavo A. R. Silva 2020-07-28 22:14 ` Winkler, Tomas 2020-07-28 22:29 ` Winkler, Tomas 2020-07-28 23:04 ` Gustavo A. R. Silva 2020-07-28 23:07 ` Winkler, Tomas 2020-07-28 23:16 ` Gustavo A. R. Silva 2020-07-29 3:57 ` Gustavo A. R. Silva 2020-07-28 23:06 ` Joe Perches 2020-07-28 23:08 ` Winkler, Tomas 2020-07-28 22:17 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox