[PATCH] drivers/usb/typec/tps6598x.c: fix two bugs

[PATCH] drivers/usb/typec/tps6598x.c: fix two bugs

[Nikolaus Voss]

1. Portinfo bit field is 3 bits wide, not 2 bits. This led to
   a wrong driver configuration for some tps6598x configurations.

2. Writing 4CC commands with tps6598x_write_4cc() already has
   a pointer arg, don't reference it when using as arg to
   tps6598x_block_write(). Correcting this enforces the constness
   of the pointer to propagate to tps6598x_block_write(), so add
   the const qualifier there to avoid the warning.

Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
---
 drivers/usb/typec/tps6598x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c674abe3cf99..a38d1409f15b 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -41,7 +41,7 @@
 #define TPS_STATUS_VCONN(s)		(!!((s) & BIT(7)))
 
 /* TPS_REG_SYSTEM_CONF bits */
-#define TPS_SYSCONF_PORTINFO(c)		((c) & 3)
+#define TPS_SYSCONF_PORTINFO(c)		((c) & 7)
 
 enum {
 	TPS_PORTINFO_SINK,
@@ -127,7 +127,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
 }
 
 static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
-				void *val, size_t len)
+				const void *val, size_t len)
 {
 	u8 data[TPS_MAX_LEN + 1];
 
@@ -173,7 +173,7 @@ static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
+	return tps6598x_block_write(tps, reg, val, 4);
 }
 
 static int tps6598x_read_partner_identity(struct tps6598x *tps)
-- 
2.17.1

Re: [PATCH] drivers/usb/typec/tps6598x.c: fix two bugs

[Greg Kroah-Hartman]

On Fri, Jun 28, 2019 at 10:12:35AM +0200, Nikolaus Voss wrote:
> 1. Portinfo bit field is 3 bits wide, not 2 bits. This led to
>    a wrong driver configuration for some tps6598x configurations.
> 
> 2. Writing 4CC commands with tps6598x_write_4cc() already has
>    a pointer arg, don't reference it when using as arg to
>    tps6598x_block_write(). Correcting this enforces the constness
>    of the pointer to propagate to tps6598x_block_write(), so add
>    the const qualifier there to avoid the warning.

Please split this up into two seprate patches as these are doing two
different things.

thanks,

greg k-h

Re: [PATCH] drivers/usb/typec/tps6598x.c: fix two bugs

[Heikki Krogerus]

On Fri, Jun 28, 2019 at 10:12:35AM +0200, Nikolaus Voss wrote:
> 1. Portinfo bit field is 3 bits wide, not 2 bits. This led to
>    a wrong driver configuration for some tps6598x configurations.
> 
> 2. Writing 4CC commands with tps6598x_write_4cc() already has
>    a pointer arg, don't reference it when using as arg to
>    tps6598x_block_write(). Correcting this enforces the constness
>    of the pointer to propagate to tps6598x_block_write(), so add
>    the const qualifier there to avoid the warning.

Like Greg said, if please supply separate patches. Also, if these are
fixes, then please mark them as such with appropriate tags:

        Fixes: ....
        Cc: stable@...


> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> ---
>  drivers/usb/typec/tps6598x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index c674abe3cf99..a38d1409f15b 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -41,7 +41,7 @@
>  #define TPS_STATUS_VCONN(s)		(!!((s) & BIT(7)))
>  
>  /* TPS_REG_SYSTEM_CONF bits */
> -#define TPS_SYSCONF_PORTINFO(c)		((c) & 3)
> +#define TPS_SYSCONF_PORTINFO(c)		((c) & 7)
>  
>  enum {
>  	TPS_PORTINFO_SINK,
> @@ -127,7 +127,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>  }
>  
>  static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> -				void *val, size_t len)
> +				const void *val, size_t len)
>  {
>  	u8 data[TPS_MAX_LEN + 1];
>  
> @@ -173,7 +173,7 @@ static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
>  static inline int
>  tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
>  {
> -	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
> +	return tps6598x_block_write(tps, reg, val, 4);
>  }
>  
>  static int tps6598x_read_partner_identity(struct tps6598x *tps)
> -- 
> 2.17.1

thanks,

-- 
heikki

[PATCH 1/2] drivers/usb/typec/tps6598x.c: fix portinfo width

[Nikolaus Voss]

Portinfo bit field is 3 bits wide, not 2 bits. This led to
a wrong driver configuration for some tps6598x configurations.

Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
---
 drivers/usb/typec/tps6598x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c674abe3cf99..a170c49c2542 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -41,7 +41,7 @@
 #define TPS_STATUS_VCONN(s)		(!!((s) & BIT(7)))
 
 /* TPS_REG_SYSTEM_CONF bits */
-#define TPS_SYSCONF_PORTINFO(c)		((c) & 3)
+#define TPS_SYSCONF_PORTINFO(c)		((c) & 7)
 
 enum {
 	TPS_PORTINFO_SINK,
-- 
2.17.1

Re: [PATCH 1/2] drivers/usb/typec/tps6598x.c: fix portinfo width

[Heikki Krogerus]

On Fri, Jun 28, 2019 at 11:01:08AM +0200, Nikolaus Voss wrote:
> Portinfo bit field is 3 bits wide, not 2 bits. This led to
> a wrong driver configuration for some tps6598x configurations.
> 
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>

Shouldn't this be applied to the stable trees as well?

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tps6598x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index c674abe3cf99..a170c49c2542 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -41,7 +41,7 @@
>  #define TPS_STATUS_VCONN(s)		(!!((s) & BIT(7)))
>  
>  /* TPS_REG_SYSTEM_CONF bits */
> -#define TPS_SYSCONF_PORTINFO(c)		((c) & 3)
> +#define TPS_SYSCONF_PORTINFO(c)		((c) & 7)
>  
>  enum {
>  	TPS_PORTINFO_SINK,
> -- 
> 2.17.1

thanks,

-- 
heikki

Re: [PATCH 1/2] drivers/usb/typec/tps6598x.c: fix portinfo width

[Nikolaus Voss]

On Fri, 28 Jun 2019, Heikki Krogerus wrote:
> On Fri, Jun 28, 2019 at 11:01:08AM +0200, Nikolaus Voss wrote:
>> Portinfo bit field is 3 bits wide, not 2 bits. This led to
>> a wrong driver configuration for some tps6598x configurations.
>>
>> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>
> Shouldn't this be applied to the stable trees as well?

Oh, yes, forgot that again... Greg, could you...?

Niko

[...]

Re: [PATCH 1/2] drivers/usb/typec/tps6598x.c: fix portinfo width

[Greg Kroah-Hartman]

On Fri, Jun 28, 2019 at 12:10:41PM +0200, Nikolaus Voss wrote:
> On Fri, 28 Jun 2019, Heikki Krogerus wrote:
> > On Fri, Jun 28, 2019 at 11:01:08AM +0200, Nikolaus Voss wrote:
> > > Portinfo bit field is 3 bits wide, not 2 bits. This led to
> > > a wrong driver configuration for some tps6598x configurations.
> > > 
> > > Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> > > Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> > 
> > Shouldn't this be applied to the stable trees as well?
> 
> Oh, yes, forgot that again... Greg, could you...?

It's not like I have anything else to do... :)

Re: [PATCH 1/2] drivers/usb/typec/tps6598x.c: fix portinfo width

[Nikolaus Voss]

On Wed, 3 Jul 2019, Greg Kroah-Hartman wrote:
> On Fri, Jun 28, 2019 at 12:10:41PM +0200, Nikolaus Voss wrote:
>> On Fri, 28 Jun 2019, Heikki Krogerus wrote:
>>> On Fri, Jun 28, 2019 at 11:01:08AM +0200, Nikolaus Voss wrote:
>>>> Portinfo bit field is 3 bits wide, not 2 bits. This led to
>>>> a wrong driver configuration for some tps6598x configurations.
>>>>
>>>> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
>>>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>>>
>>> Shouldn't this be applied to the stable trees as well?
>>
>> Oh, yes, forgot that again... Greg, could you...?
>
> It's not like I have anything else to do... :)

I know. Sorry. I'll repost if you want, just say ick or something then...

Nikolaus

Re: [PATCH 1/2] drivers/usb/typec/tps6598x.c: fix portinfo width

[Greg Kroah-Hartman]

On Wed, Jul 03, 2019 at 07:25:53PM +0200, Nikolaus Voss wrote:
> On Wed, 3 Jul 2019, Greg Kroah-Hartman wrote:
> > On Fri, Jun 28, 2019 at 12:10:41PM +0200, Nikolaus Voss wrote:
> > > On Fri, 28 Jun 2019, Heikki Krogerus wrote:
> > > > On Fri, Jun 28, 2019 at 11:01:08AM +0200, Nikolaus Voss wrote:
> > > > > Portinfo bit field is 3 bits wide, not 2 bits. This led to
> > > > > a wrong driver configuration for some tps6598x configurations.
> > > > > 
> > > > > Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> > > > > Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> > > > 
> > > > Shouldn't this be applied to the stable trees as well?
> > > 
> > > Oh, yes, forgot that again... Greg, could you...?
> > 
> > It's not like I have anything else to do... :)
> 
> I know. Sorry. I'll repost if you want, just say ick or something then...

Already merged, no worries.

[PATCH 2/2] drivers/usb/typec/tps6598x.c: fix 4CC cmd write

[Nikolaus Voss]

Writing 4CC commands with tps6598x_write_4cc() already has
a pointer arg, don't reference it when using as arg to
tps6598x_block_write(). Correcting this enforces the constness
of the pointer to propagate to tps6598x_block_write(), so add
the const qualifier there to avoid the warning.

Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
---
 drivers/usb/typec/tps6598x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index a170c49c2542..a38d1409f15b 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -127,7 +127,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
 }
 
 static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
-				void *val, size_t len)
+				const void *val, size_t len)
 {
 	u8 data[TPS_MAX_LEN + 1];
 
@@ -173,7 +173,7 @@ static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
+	return tps6598x_block_write(tps, reg, val, 4);
 }
 
 static int tps6598x_read_partner_identity(struct tps6598x *tps)
-- 
2.17.1

Re: [PATCH 2/2] drivers/usb/typec/tps6598x.c: fix 4CC cmd write

[Heikki Krogerus]

On Fri, Jun 28, 2019 at 11:01:09AM +0200, Nikolaus Voss wrote:
> Writing 4CC commands with tps6598x_write_4cc() already has
> a pointer arg, don't reference it when using as arg to
> tps6598x_block_write(). Correcting this enforces the constness
> of the pointer to propagate to tps6598x_block_write(), so add
> the const qualifier there to avoid the warning.
> 
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tps6598x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index a170c49c2542..a38d1409f15b 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -127,7 +127,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>  }
>  
>  static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> -				void *val, size_t len)
> +				const void *val, size_t len)
>  {
>  	u8 data[TPS_MAX_LEN + 1];
>  
> @@ -173,7 +173,7 @@ static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
>  static inline int
>  tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
>  {
> -	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
> +	return tps6598x_block_write(tps, reg, val, 4);
>  }
>  
>  static int tps6598x_read_partner_identity(struct tps6598x *tps)
> -- 
> 2.17.1

thanks,

-- 
heikki