[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Greg Kroah-Hartman]

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> adapters, but the problem described with regmap-i2c not handling
> SMBus block transfers (i.e. read and writes) correctly also exists
> with writes.
> 
> As workaround, this patch adds a block write function the same way
> 1a2f474d328f adds a block read function.
> 
> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> ---

As was pointed out, you have to have a From: that matches a
signed-off-by somewhere here.  If your company email systems is horrid
and can not handle patches, then put the correct from: line as the first
line of the commit message as the documentation says and all will be
good.



> v2: fix tps6598x_exec_cmd also
> ---
>  drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index c84c8c189e90..c54b73fb2a2f 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>  	return 0;
>  }
>  
> +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> +				void *val, size_t len)
> +{
> +	u8 data[len + 1];

I thought the build system now warned when you did this :(

thanks,

greg k-h

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[kbuild test robot]

Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.0-rc4 next-20190220]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nikolaus-Voss/usb-typec-tps6598x-handle-block-writes-separately-with-plain-I2C-adapters/20190221-165456
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-a0-201907 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/usb/typec/tps6598x.c: In function 'tps6598x_block_write':
>> drivers/usb/typec/tps6598x.c:132:2: warning: variable length array 'data' is used [-Wvla]
     u8 data[len + 1];
     ^

vim +/data +132 drivers/usb/typec/tps6598x.c

   128	
   129	static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
   130					void *val, size_t len)
   131	{
 > 132		u8 data[len + 1];
   133	
   134		if (!tps->i2c_protocol)
   135			return regmap_raw_write(tps->regmap, reg, val, len);
   136	
   137		data[0] = len;
   138		memcpy(&data[1], val, len);
   139	
   140		return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
   141	}
   142
---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Greg Kroah-Hartman]

On Thu, Feb 21, 2019 at 09:37:33AM +0100, Nikolaus Voss wrote:
> Hi Greg,
> 
> On Wed, 20 Feb 2019, Greg Kroah-Hartman wrote:
> > On Wed, Feb 20, 2019 at 04:22:00PM +0100, Nikolaus Voss wrote:
> > > > > v2: fix tps6598x_exec_cmd also
> > > > > ---
> > > > >  drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
> > > > >  1 file changed, 20 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> > > > > index c84c8c189e90..c54b73fb2a2f 100644
> > > > > --- a/drivers/usb/typec/tps6598x.c
> > > > > +++ b/drivers/usb/typec/tps6598x.c
> > > > > @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
> > > > >  	return 0;
> > > > >  }
> > > > > 
> > > > > +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> > > > > +				void *val, size_t len)
> > > > > +{
> > > > > +	u8 data[len + 1];
> > > > 
> > > > I thought the build system now warned when you did this :(
> > > 
> > > I must admit I'm developing on 4.19 stable series, so no warnings...
> > 
> > Ick, no, you are 6 months behind where the rest of us are :(
> > 
> > Always, at the very least, work off of Linus's tree.  For best results,
> > work off of linux-next.
> 
> we are a medical device manufacturer and our prototypes run stable kernels
> because our main development goal is the patient therapy.

That's great, and fine, but testing on newer kernels is always a good
idea, that way you are not over a year behind when you have to move to
the next LTS release :)

thanks,

greg k-h

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

Hi Greg,

On Wed, 20 Feb 2019, Greg Kroah-Hartman wrote:
> On Wed, Feb 20, 2019 at 04:22:00PM +0100, Nikolaus Voss wrote:
>>>> v2: fix tps6598x_exec_cmd also
>>>> ---
>>>>  drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
>>>>  1 file changed, 20 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
>>>> index c84c8c189e90..c54b73fb2a2f 100644
>>>> --- a/drivers/usb/typec/tps6598x.c
>>>> +++ b/drivers/usb/typec/tps6598x.c
>>>> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>>>>  	return 0;
>>>>  }
>>>>
>>>> +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
>>>> +				void *val, size_t len)
>>>> +{
>>>> +	u8 data[len + 1];
>>>
>>> I thought the build system now warned when you did this :(
>>
>> I must admit I'm developing on 4.19 stable series, so no warnings...
>
> Ick, no, you are 6 months behind where the rest of us are :(
>
> Always, at the very least, work off of Linus's tree.  For best results,
> work off of linux-next.

we are a medical device manufacturer and our prototypes run stable 
kernels because our main development goal is the patient therapy.

However, what I do check is that my patches apply cleanly onto Linus's 
tree and if I see any other changes of files my patch touches I rebase and 
compile.

I'll try to always do the last before I submit a patch in the future but 
testing can only be done on our prototype branch (with reasonable effort).

Nikolaus

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Greg Kroah-Hartman]

On Wed, Feb 20, 2019 at 04:22:00PM +0100, Nikolaus Voss wrote:
> > > v2: fix tps6598x_exec_cmd also
> > > ---
> > >  drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
> > >  1 file changed, 20 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> > > index c84c8c189e90..c54b73fb2a2f 100644
> > > --- a/drivers/usb/typec/tps6598x.c
> > > +++ b/drivers/usb/typec/tps6598x.c
> > > @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
> > >  	return 0;
> > >  }
> > > 
> > > +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> > > +				void *val, size_t len)
> > > +{
> > > +	u8 data[len + 1];
> > 
> > I thought the build system now warned when you did this :(
> 
> I must admit I'm developing on 4.19 stable series, so no warnings...

Ick, no, you are 6 months behind where the rest of us are :(

Always, at the very least, work off of Linus's tree.  For best results,
work off of linux-next.

thanks,

greg k-h

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Greg Kroah-Hartman wrote:
> On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
>> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
>> adapters, but the problem described with regmap-i2c not handling
>> SMBus block transfers (i.e. read and writes) correctly also exists
>> with writes.
>>
>> As workaround, this patch adds a block write function the same way
>> 1a2f474d328f adds a block read function.
>>
>> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
>> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>> ---
>
> As was pointed out, you have to have a From: that matches a
> signed-off-by somewhere here.  If your company email systems is horrid
> and can not handle patches, then put the correct from: line as the first
> line of the commit message as the documentation says and all will be
> good.
>
>
>
>> v2: fix tps6598x_exec_cmd also
>> ---
>>  drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
>>  1 file changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
>> index c84c8c189e90..c54b73fb2a2f 100644
>> --- a/drivers/usb/typec/tps6598x.c
>> +++ b/drivers/usb/typec/tps6598x.c
>> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>>  	return 0;
>>  }
>>
>> +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
>> +				void *val, size_t len)
>> +{
>> +	u8 data[len + 1];
>
> I thought the build system now warned when you did this :(

I must admit I'm developing on 4.19 stable series, so no warnings...

Nikolaus

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Guenter Roeck wrote:
> On 2/20/19 4:57 AM, Nikolaus Voss wrote:
>> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
>> adapters, but the problem described with regmap-i2c not handling
>> SMBus block transfers (i.e. read and writes) correctly also exists
>> with writes.
>> 
>> As workaround, this patch adds a block write function the same way
>> 1a2f474d328f adds a block read function.
>> 
>> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately 
>> with plain-I2C adapters")
>> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery 
>> controllers")
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>> ---
>> v2: fix tps6598x_exec_cmd also
>> ---
>>   drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
>>   1 file changed, 20 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
>> index c84c8c189e90..c54b73fb2a2f 100644
>> --- a/drivers/usb/typec/tps6598x.c
>> +++ b/drivers/usb/typec/tps6598x.c
>> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void 
>> *val, size_t len)
>>   	return 0;
>>   }
>>   +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
>> +				void *val, size_t len)
>> +{
>> +	u8 data[len + 1];
>> +
>
> You should use TPS_MAX_LEN + 1 here to avoid the variable length array.
> See upstream commit 0bb95f80a38f8 ("Makefile: Globally enable VLA warning")
> and 8d361fa2c29dc ("usb: typec: tps6598x: Remove VLA usage"). Not sure if
> the WARN_ON introduced by 8d361fa2c29dc is really needed; I dislike
> unnecessary runtime checks.

Thanks for the pointer, I fixed this...

Nikolaus

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Heikki Krogerus wrote:
> On Wed, Feb 20, 2019 at 04:14:23PM +0200, Heikki Krogerus wrote:
>> On Wed, Feb 20, 2019 at 02:38:47PM +0100, Nikolaus Voss wrote:
>>> On Wed, 20 Feb 2019, Heikki Krogerus wrote:
>>>> On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
>>>>> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
>>>>> adapters, but the problem described with regmap-i2c not handling
>>>>> SMBus block transfers (i.e. read and writes) correctly also exists
>>>>> with writes.
>>>>>
>>>>> As workaround, this patch adds a block write function the same way
>>>>> 1a2f474d328f adds a block read function.
>>>>>
>>>>> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
>>>>> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
>>>>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>>>>
>>>> You are missing a "from" line with address that matches your SoB
>>>> address.
>>>
>>> That's because I currently cannot send patch mails from my company account
>>> as our MTA breaks diffs.
>>
>> I understand, but you can have a separate "From line" in your patch,
>> i.e. you send the patch using one address, and have an extra "From
>> line" (outside of the mail header) with another address.
>>
>> That other From line will be interpreted as the author address, and
>> it should match your SoB address.
>>
>>
>> Try something like this in a branch where this patch is the HEAD:
>>
>>         % export MY_COMMIT=$(git show -s --pretty=%h HEAD)
>>         % git reset HEAD^
>>         % GIT_COMMITTER_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
>>           GIT_AUTHOR_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
>>           git commit -a -C $MY_COMMIT
>
> Correction here:
>
>        % GIT_COMMITTER_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
>          GIT_AUTHOR_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
>          git commit -a -C $MY_COMMIT --reset-author
>
> That "--reset-author" was missing. Sorry for that.

Thanks, Heikki, I'll give it a try...

Nikolaus

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Guenter Roeck]

On 2/20/19 4:57 AM, Nikolaus Voss wrote:
> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> adapters, but the problem described with regmap-i2c not handling
> SMBus block transfers (i.e. read and writes) correctly also exists
> with writes.
> 
> As workaround, this patch adds a block write function the same way
> 1a2f474d328f adds a block read function.
> 
> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> ---
> v2: fix tps6598x_exec_cmd also
> ---
>   drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
>   1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index c84c8c189e90..c54b73fb2a2f 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>   	return 0;
>   }
>   
> +static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> +				void *val, size_t len)
> +{
> +	u8 data[len + 1];
> +

You should use TPS_MAX_LEN + 1 here to avoid the variable length array.
See upstream commit 0bb95f80a38f8 ("Makefile: Globally enable VLA warning")
and 8d361fa2c29dc ("usb: typec: tps6598x: Remove VLA usage"). Not sure if
the WARN_ON introduced by 8d361fa2c29dc is really needed; I dislike
unnecessary runtime checks.

Guenter

> +	if (!tps->i2c_protocol)
> +		return regmap_raw_write(tps->regmap, reg, val, len);
> +
> +	data[0] = len;
> +	memcpy(&data[1], val, len);
> +
> +	return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
> +}
> +
>   static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
>   {
>   	return tps6598x_block_read(tps, reg, val, sizeof(u16));
> @@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, u8 reg, u64 *val)
>   
>   static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
>   {
> -	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
> +	return tps6598x_block_write(tps, reg, &val, sizeof(u16));
>   }
>   
>   static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
>   {
> -	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
> +	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
>   }
>   
>   static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
>   {
> -	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
> +	return tps6598x_block_write(tps, reg, &val, sizeof(u64));
>   }
>   
>   static inline int
>   tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
>   {
> -	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
> +	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
>   }
>   
>   static int tps6598x_read_partner_identity(struct tps6598x *tps)
> @@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
>   		return -EBUSY;
>   
>   	if (in_len) {
> -		ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
> -				       in_data, in_len);
> +		ret = tps6598x_block_write(tps, TPS_REG_DATA1,
> +					   in_data, in_len);
>   		if (ret)
>   			return ret;
>   	}
>

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

On Wed, Feb 20, 2019 at 04:14:23PM +0200, Heikki Krogerus wrote:
> On Wed, Feb 20, 2019 at 02:38:47PM +0100, Nikolaus Voss wrote:
> > On Wed, 20 Feb 2019, Heikki Krogerus wrote:
> > > On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> > > > Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> > > > adapters, but the problem described with regmap-i2c not handling
> > > > SMBus block transfers (i.e. read and writes) correctly also exists
> > > > with writes.
> > > > 
> > > > As workaround, this patch adds a block write function the same way
> > > > 1a2f474d328f adds a block read function.
> > > > 
> > > > Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
> > > > Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> > > > Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> > > 
> > > You are missing a "from" line with address that matches your SoB
> > > address.
> > 
> > That's because I currently cannot send patch mails from my company account
> > as our MTA breaks diffs.
> 
> I understand, but you can have a separate "From line" in your patch,
> i.e. you send the patch using one address, and have an extra "From
> line" (outside of the mail header) with another address.
> 
> That other From line will be interpreted as the author address, and
> it should match your SoB address.
> 
> 
> Try something like this in a branch where this patch is the HEAD:
> 
>         % export MY_COMMIT=$(git show -s --pretty=%h HEAD)
>         % git reset HEAD^
>         % GIT_COMMITTER_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
>           GIT_AUTHOR_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
>           git commit -a -C $MY_COMMIT

Correction here:

        % GIT_COMMITTER_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
          GIT_AUTHOR_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
          git commit -a -C $MY_COMMIT --reset-author

That "--reset-author" was missing. Sorry for that.


thanks,

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

On Wed, Feb 20, 2019 at 02:38:47PM +0100, Nikolaus Voss wrote:
> On Wed, 20 Feb 2019, Heikki Krogerus wrote:
> > On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> > > Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> > > adapters, but the problem described with regmap-i2c not handling
> > > SMBus block transfers (i.e. read and writes) correctly also exists
> > > with writes.
> > > 
> > > As workaround, this patch adds a block write function the same way
> > > 1a2f474d328f adds a block read function.
> > > 
> > > Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
> > > Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> > > Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> > 
> > You are missing a "from" line with address that matches your SoB
> > address.
> 
> That's because I currently cannot send patch mails from my company account
> as our MTA breaks diffs.

I understand, but you can have a separate "From line" in your patch,
i.e. you send the patch using one address, and have an extra "From
line" (outside of the mail header) with another address.

That other From line will be interpreted as the author address, and
it should match your SoB address.


Try something like this in a branch where this patch is the HEAD:

        % export MY_COMMIT=$(git show -s --pretty=%h HEAD)
        % git reset HEAD^
        % GIT_COMMITTER_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
          GIT_AUTHOR_IDENT='Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>' \
          git commit -a -C $MY_COMMIT

Then:

        % git format-patch HEAD^
        % git send-email ...

thanks,

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

On Wed, 20 Feb 2019, Heikki Krogerus wrote:
> On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
>> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
>> adapters, but the problem described with regmap-i2c not handling
>> SMBus block transfers (i.e. read and writes) correctly also exists
>> with writes.
>>
>> As workaround, this patch adds a block write function the same way
>> 1a2f474d328f adds a block read function.
>>
>> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
>> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>
> You are missing a "from" line with address that matches your SoB
> address.

That's because I currently cannot send patch mails from my company 
account as our MTA breaks diffs. You could add

Signed-off-by: Nikolaus Voss <nv@vosn.de>

Nikolaus

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Heikki Krogerus]

On Wed, Feb 20, 2019 at 01:57:30PM +0100, Nikolaus Voss wrote:
> Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
> adapters, but the problem described with regmap-i2c not handling
> SMBus block transfers (i.e. read and writes) correctly also exists
> with writes.
> 
> As workaround, this patch adds a block write function the same way
> 1a2f474d328f adds a block read function.
> 
> Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>

You are missing a "from" line with address that matches your SoB
address.


thanks,

[PATCHv2] usb: typec: tps6598x: handle block writes separately with plain-I2C adapters

[Nikolaus Voss]

Commit 1a2f474d328f handles block _reads_ separately with plain-I2C
adapters, but the problem described with regmap-i2c not handling
SMBus block transfers (i.e. read and writes) correctly also exists
with writes.

As workaround, this patch adds a block write function the same way
1a2f474d328f adds a block read function.

Fixes: 1a2f474d328f ("usb: typec: tps6598x: handle block reads separately with plain-I2C adapters")
Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
---
v2: fix tps6598x_exec_cmd also
---
 drivers/usb/typec/tps6598x.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index c84c8c189e90..c54b73fb2a2f 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -110,6 +110,20 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
 	return 0;
 }
 
+static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
+				void *val, size_t len)
+{
+	u8 data[len + 1];
+
+	if (!tps->i2c_protocol)
+		return regmap_raw_write(tps->regmap, reg, val, len);
+
+	data[0] = len;
+	memcpy(&data[1], val, len);
+
+	return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+}
+
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
 {
 	return tps6598x_block_read(tps, reg, val, sizeof(u16));
@@ -127,23 +141,23 @@ static inline int tps6598x_read64(struct tps6598x *tps, u8 reg, u64 *val)
 
 static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
 {
-	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u16));
+	return tps6598x_block_write(tps, reg, &val, sizeof(u16));
 }
 
 static inline int tps6598x_write32(struct tps6598x *tps, u8 reg, u32 val)
 {
-	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
 {
-	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u64));
+	return tps6598x_block_write(tps, reg, &val, sizeof(u64));
 }
 
 static inline int
 tps6598x_write_4cc(struct tps6598x *tps, u8 reg, const char *val)
 {
-	return regmap_raw_write(tps->regmap, reg, &val, sizeof(u32));
+	return tps6598x_block_write(tps, reg, &val, sizeof(u32));
 }
 
 static int tps6598x_read_partner_identity(struct tps6598x *tps)
@@ -229,8 +243,8 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
 		return -EBUSY;
 
 	if (in_len) {
-		ret = regmap_raw_write(tps->regmap, TPS_REG_DATA1,
-				       in_data, in_len);
+		ret = tps6598x_block_write(tps, TPS_REG_DATA1,
+					   in_data, in_len);
 		if (ret)
 			return ret;
 	}