Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1712221 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] thunderbolt: fix incorrect value assigned to req->response_type Colin King <colin.king@canonical.com> - 2017-08-15 16:40 +0200
    Re: [PATCH] thunderbolt: fix incorrect value assigned to  req->response_type Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-08-15 16:40 +0200
      Re: [PATCH] thunderbolt: fix incorrect value assigned to  req->response_type Dan Carpenter <dan.carpenter@oracle.com> - 2017-08-15 17:30 +0200
        Re: [PATCH] thunderbolt: fix incorrect value assigned to  req->response_type Colin Ian King <colin.king@canonical.com> - 2017-08-15 17:50 +0200
          [PATCH resend] thunderbolt: Fix reset response_type Dan Carpenter <dan.carpenter@oracle.com> - 2017-08-16 11:00 +0200
        Re: [PATCH] thunderbolt: fix incorrect value assigned to  req->response_type Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-08-16 10:20 +0200
          Re: [PATCH] thunderbolt: fix incorrect value assigned to  req->response_type Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-08-16 18:20 +0200
            Re: [PATCH] thunderbolt: fix incorrect value assigned to  req->response_type Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-08-17 11:50 +0200

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

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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
> 

[toc] | [prev] | [next] | [standalone]

#1712778 — [PATCH resend] thunderbolt: Fix reset response_type

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);
 

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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 :)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web