[PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
and should actually be assigned TB_CFG_PKG_RESET. Fix this.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/thunderbolt/ctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 69c0232a22f8..fb40dd0588b9 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -804,7 +804,7 @@ struct tb_cfg_result tb_cfg_reset(struct tb_ctl *ctl, u64 route,
 	req->request_type = TB_CFG_PKG_RESET;
 	req->response = &reply;
 	req->response_size = sizeof(reply);
-	req->response_type = sizeof(TB_CFG_PKG_RESET);
+	req->response_type = TB_CFG_PKG_RESET;
 
 	res = tb_cfg_request_sync(ctl, req, timeout_msec);
 
-- 
2.11.0

Re: [PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Mika Westerberg]

On Tue, Aug 15, 2017 at 03:31:33PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
> and should actually be assigned TB_CFG_PKG_RESET. Fix this.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

IIRC I already acked this some time ago ;-)

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

BTW, you should CC Greg as he has been gathering Thunderbolt related
patches. I added him now.

> ---
>  drivers/thunderbolt/ctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
> index 69c0232a22f8..fb40dd0588b9 100644
> --- a/drivers/thunderbolt/ctl.c
> +++ b/drivers/thunderbolt/ctl.c
> @@ -804,7 +804,7 @@ struct tb_cfg_result tb_cfg_reset(struct tb_ctl *ctl, u64 route,
>  	req->request_type = TB_CFG_PKG_RESET;
>  	req->response = &reply;
>  	req->response_size = sizeof(reply);
> -	req->response_type = sizeof(TB_CFG_PKG_RESET);
> +	req->response_type = TB_CFG_PKG_RESET;
>  
>  	res = tb_cfg_request_sync(ctl, req, timeout_msec);
>  
> -- 
> 2.11.0

Re: [PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Dan Carpenter]

On Tue, Aug 15, 2017 at 05:38:34PM +0300, Mika Westerberg wrote:
> On Tue, Aug 15, 2017 at 03:31:33PM +0100, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> > 
> > req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
> > and should actually be assigned TB_CFG_PKG_RESET. Fix this.
> > 
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> IIRC I already acked this some time ago ;-)
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> BTW, you should CC Greg as he has been gathering Thunderbolt related
> patches. I added him now.

Yeah.  I sent this patch on Jun 14.  "[PATCH] thunderbolt: Fix reset
response_type"  I didn't CC Greg, either.  Someone should probably
update MAINTAINERS if Greg needs to get these emails.

Colin, could you please add Fixes tags to your patches?  It helps me
investigate how bugs are introduced.  I know we all had a long thread
and agreed to not add them if the patch probably doesn't fix a user
visible bug, but this one is clearly a bugfix.

regards,
dan carpenter

Re: [PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Colin Ian King]

On 15/08/17 16:22, Dan Carpenter wrote:
> On Tue, Aug 15, 2017 at 05:38:34PM +0300, Mika Westerberg wrote:
>> On Tue, Aug 15, 2017 at 03:31:33PM +0100, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
>>> and should actually be assigned TB_CFG_PKG_RESET. Fix this.
>>>
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>
>> IIRC I already acked this some time ago ;-)
>>
>> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>
>> BTW, you should CC Greg as he has been gathering Thunderbolt related
>> patches. I added him now.
> 
> Yeah.  I sent this patch on Jun 14.  "[PATCH] thunderbolt: Fix reset
> response_type"  I didn't CC Greg, either.  Someone should probably
> update MAINTAINERS if Greg needs to get these emails.
> 
> Colin, could you please add Fixes tags to your patches? 

Will do.

> It helps me
> investigate how bugs are introduced.  I know we all had a long thread
> and agreed to not add them if the patch probably doesn't fix a user
> visible bug, but this one is clearly a bugfix.

Since you fixed this back in Jun, will you re-send your original fix?

Colin
> 
> regards,
> dan carpenter
> 

[PATCH resend] thunderbolt: Fix reset response_type

[Dan Carpenter]

There is a mistake here where we accidentally use sizeof(TB_CFG_PKG_RESET)
instead of just TB_CFG_PKG_RESET.  The size of an int is 4 so it's the
same as TB_CFG_PKG_NOTIFY_ACK.

Fixes: d7f781bfdbf4 ("thunderbolt: Rework control channel to be more reliable")
Reported-by: Colin King <colin.king@canonical.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Resending with Greg CC'd.  This was also:
Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
But I wasn't sure if I should add that tag.

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 69c0232a22f8..fb40dd0588b9 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -804,7 +804,7 @@ struct tb_cfg_result tb_cfg_reset(struct tb_ctl *ctl, u64 route,
 	req->request_type = TB_CFG_PKG_RESET;
 	req->response = &reply;
 	req->response_size = sizeof(reply);
-	req->response_type = sizeof(TB_CFG_PKG_RESET);
+	req->response_type = TB_CFG_PKG_RESET;
 
 	res = tb_cfg_request_sync(ctl, req, timeout_msec);
 

Re: [PATCH resend] thunderbolt: Fix reset response_type

[Mika Westerberg]

On Wed, Aug 16, 2017 at 11:54:17AM +0300, Dan Carpenter wrote:
> There is a mistake here where we accidentally use sizeof(TB_CFG_PKG_RESET)
> instead of just TB_CFG_PKG_RESET.  The size of an int is 4 so it's the
> same as TB_CFG_PKG_NOTIFY_ACK.
> 
> Fixes: d7f781bfdbf4 ("thunderbolt: Rework control channel to be more reliable")
> Reported-by: Colin King <colin.king@canonical.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> Resending with Greg CC'd.  This was also:
> Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
> But I wasn't sure if I should add that tag.

Hi Greg,

Can you pick this fix to your char-misc tree?

Thanks!

Re: [PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Mika Westerberg]

On Tue, Aug 15, 2017 at 06:22:59PM +0300, Dan Carpenter wrote:
> On Tue, Aug 15, 2017 at 05:38:34PM +0300, Mika Westerberg wrote:
> > On Tue, Aug 15, 2017 at 03:31:33PM +0100, Colin King wrote:
> > > From: Colin Ian King <colin.king@canonical.com>
> > > 
> > > req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
> > > and should actually be assigned TB_CFG_PKG_RESET. Fix this.
> > > 
> > > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > 
> > IIRC I already acked this some time ago ;-)
> > 
> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 
> > BTW, you should CC Greg as he has been gathering Thunderbolt related
> > patches. I added him now.
> 
> Yeah.  I sent this patch on Jun 14.  "[PATCH] thunderbolt: Fix reset
> response_type"  I didn't CC Greg, either.  Someone should probably
> update MAINTAINERS if Greg needs to get these emails.

Indeed.

Greg, Andreas,

Should we add Greg to the MAINTAINERS so that he will be getting all the
thunderbolt related patches? Another way, if we want to make this easier
for Greg, is to establish a thunderbolt tree in kernel.org and send pull
requests to him directly.

Please let me know your preference.

Re: [PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Greg Kroah-Hartman]

On Wed, Aug 16, 2017 at 11:10:01AM +0300, Mika Westerberg wrote:
> On Tue, Aug 15, 2017 at 06:22:59PM +0300, Dan Carpenter wrote:
> > On Tue, Aug 15, 2017 at 05:38:34PM +0300, Mika Westerberg wrote:
> > > On Tue, Aug 15, 2017 at 03:31:33PM +0100, Colin King wrote:
> > > > From: Colin Ian King <colin.king@canonical.com>
> > > > 
> > > > req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
> > > > and should actually be assigned TB_CFG_PKG_RESET. Fix this.
> > > > 
> > > > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > > 
> > > IIRC I already acked this some time ago ;-)
> > > 
> > > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > 
> > > BTW, you should CC Greg as he has been gathering Thunderbolt related
> > > patches. I added him now.
> > 
> > Yeah.  I sent this patch on Jun 14.  "[PATCH] thunderbolt: Fix reset
> > response_type"  I didn't CC Greg, either.  Someone should probably
> > update MAINTAINERS if Greg needs to get these emails.
> 
> Indeed.
> 
> Greg, Andreas,
> 
> Should we add Greg to the MAINTAINERS so that he will be getting all the
> thunderbolt related patches? Another way, if we want to make this easier
> for Greg, is to establish a thunderbolt tree in kernel.org and send pull
> requests to him directly.

The "maintainers" of thunderbolt should either be forwarding on patches
to me to accept, or to give me a git tree to pull, or to ack patches
that I am cc:ed on.  Whichever works best for them, but in all cases, I
am not the maintainer of the thunderbolt subsystem, so I don't need to
be listed under MAINTAINERS.

thanks,

greg k-h

Re: [PATCH] thunderbolt: fix incorrect value assigned to req->response_type

[Mika Westerberg]

On Wed, Aug 16, 2017 at 09:13:27AM -0700, Greg Kroah-Hartman wrote:
> On Wed, Aug 16, 2017 at 11:10:01AM +0300, Mika Westerberg wrote:
> > On Tue, Aug 15, 2017 at 06:22:59PM +0300, Dan Carpenter wrote:
> > > On Tue, Aug 15, 2017 at 05:38:34PM +0300, Mika Westerberg wrote:
> > > > On Tue, Aug 15, 2017 at 03:31:33PM +0100, Colin King wrote:
> > > > > From: Colin Ian King <colin.king@canonical.com>
> > > > > 
> > > > > req->response_type is being assigned the sizeof TB_CFG_PKG_RESET
> > > > > and should actually be assigned TB_CFG_PKG_RESET. Fix this.
> > > > > 
> > > > > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > > > 
> > > > IIRC I already acked this some time ago ;-)
> > > > 
> > > > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > > 
> > > > BTW, you should CC Greg as he has been gathering Thunderbolt related
> > > > patches. I added him now.
> > > 
> > > Yeah.  I sent this patch on Jun 14.  "[PATCH] thunderbolt: Fix reset
> > > response_type"  I didn't CC Greg, either.  Someone should probably
> > > update MAINTAINERS if Greg needs to get these emails.
> > 
> > Indeed.
> > 
> > Greg, Andreas,
> > 
> > Should we add Greg to the MAINTAINERS so that he will be getting all the
> > thunderbolt related patches? Another way, if we want to make this easier
> > for Greg, is to establish a thunderbolt tree in kernel.org and send pull
> > requests to him directly.
> 
> The "maintainers" of thunderbolt should either be forwarding on patches
> to me to accept, or to give me a git tree to pull, or to ack patches
> that I am cc:ed on.  Whichever works best for them, but in all cases, I
> am not the maintainer of the thunderbolt subsystem, so I don't need to
> be listed under MAINTAINERS.

OK, thanks for the clarification :)