mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC] commands: i2c_write: enable raw write to address
@ 2018-02-08  7:48 Antony Pavlov
  2018-02-09  8:36 ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: Antony Pavlov @ 2018-02-08  7:48 UTC (permalink / raw)
  To: barebox

Sometimes for communication with a simple I2C devices
(e.g. PCF8574 or TM1650) it's necessary to send only
one data byte into the I2C device.
Current i2c_write command makes this impossible because
you can't just pass 'device address' and 'register number'
(or 'device address' and 'one data byte') to the command.
You always have to pass all three parameters:
'device address', 'register number' and 'data'.

This commit fixes the problem.

Sample usage:

  barebox@barebox sandbox:/ i2c_write -a 0x24 0x01

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 commands/i2c.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/commands/i2c.c b/commands/i2c.c
index b74c53509f..21c39fe5af 100644
--- a/commands/i2c.c
+++ b/commands/i2c.c
@@ -115,7 +115,7 @@ static int do_i2c_write(int argc, char *argv[])
 
 	count = argc - optind;
 
-	if ((addr < 0) || (reg < 0) || (count == 0) || (addr > 0x7F))
+	if ((addr < 0) || (count == 0) || (addr > 0x7F))
 		return COMMAND_ERROR_USAGE;
 
 	adapter = i2c_get_adapter(bus);
@@ -131,7 +131,11 @@ static int do_i2c_write(int argc, char *argv[])
 	for (i = 0; i < count; i++)
 		*(buf + i) = (char) simple_strtol(argv[optind+i], NULL, 0);
 
-	ret = i2c_write_reg(&client, reg | wide, buf, count);
+	if (reg > 0) {
+		ret = i2c_write_reg(&client, reg | wide, buf, count);
+	} else {
+		ret = i2c_master_send(&client, buf, count);
+	}
 	if (ret != count) {
 		if (verbose)
 			printf("write aborted, count(%i) != writestatus(%i)\n",
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] commands: i2c_write: enable raw write to address
  2018-02-08  7:48 [RFC] commands: i2c_write: enable raw write to address Antony Pavlov
@ 2018-02-09  8:36 ` Sascha Hauer
  2018-02-09 11:22   ` Antony Pavlov
  0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2018-02-09  8:36 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

Hi Antony,

On Thu, Feb 08, 2018 at 10:48:56AM +0300, Antony Pavlov wrote:
> Sometimes for communication with a simple I2C devices
> (e.g. PCF8574 or TM1650) it's necessary to send only
> one data byte into the I2C device.
> Current i2c_write command makes this impossible because
> you can't just pass 'device address' and 'register number'
> (or 'device address' and 'one data byte') to the command.
> You always have to pass all three parameters:
> 'device address', 'register number' and 'data'.
> 
> This commit fixes the problem.
> 
> Sample usage:
> 
>   barebox@barebox sandbox:/ i2c_write -a 0x24 0x01
> 
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  commands/i2c.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Looks good to me.

I applied this one despite the [RFC] tag. If for some reason you want to
resend this, feel free to do so, but otherwise consider this applied.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] commands: i2c_write: enable raw write to address
  2018-02-09  8:36 ` Sascha Hauer
@ 2018-02-09 11:22   ` Antony Pavlov
  2018-02-13  8:19     ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: Antony Pavlov @ 2018-02-09 11:22 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Fri, 9 Feb 2018 09:36:36 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Antony,
> 
> On Thu, Feb 08, 2018 at 10:48:56AM +0300, Antony Pavlov wrote:
> > Sometimes for communication with a simple I2C devices
> > (e.g. PCF8574 or TM1650) it's necessary to send only
> > one data byte into the I2C device.
> > Current i2c_write command makes this impossible because
> > you can't just pass 'device address' and 'register number'
> > (or 'device address' and 'one data byte') to the command.
> > You always have to pass all three parameters:
> > 'device address', 'register number' and 'data'.
> > 
> > This commit fixes the problem.
> > 
> > Sample usage:
> > 
> >   barebox@barebox sandbox:/ i2c_write -a 0x24 0x01
> > 
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  commands/i2c.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Looks good to me.
> 
> I applied this one despite the [RFC] tag. If for some reason you want to
> resend this, feel free to do so, but otherwise consider this applied.

Thanks!

The patch is short, it's understandable and it fixes the problem.
I prefere to apply this patch as is.

I see some inconsistency in drivers/i2c/i2c.c.
We have i2c_master_send() and i2c_write_reg() functions.
These functions are intended to make similar work but they are
written in very different style. I suppose that we can rewrite
i2c_master_send() (e.g. drop FIXME) and make i2c_write_reg()
work on top of i2c_master_send(). Any comments?

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] commands: i2c_write: enable raw write to address
  2018-02-09 11:22   ` Antony Pavlov
@ 2018-02-13  8:19     ` Sascha Hauer
  2018-02-14  6:19       ` Antony Pavlov
  0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2018-02-13  8:19 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

On Fri, Feb 09, 2018 at 02:22:24PM +0300, Antony Pavlov wrote:
> On Fri, 9 Feb 2018 09:36:36 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi Antony,
> > 
> > On Thu, Feb 08, 2018 at 10:48:56AM +0300, Antony Pavlov wrote:
> > > Sometimes for communication with a simple I2C devices
> > > (e.g. PCF8574 or TM1650) it's necessary to send only
> > > one data byte into the I2C device.
> > > Current i2c_write command makes this impossible because
> > > you can't just pass 'device address' and 'register number'
> > > (or 'device address' and 'one data byte') to the command.
> > > You always have to pass all three parameters:
> > > 'device address', 'register number' and 'data'.
> > > 
> > > This commit fixes the problem.
> > > 
> > > Sample usage:
> > > 
> > >   barebox@barebox sandbox:/ i2c_write -a 0x24 0x01
> > > 
> > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > ---
> > >  commands/i2c.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > Looks good to me.
> > 
> > I applied this one despite the [RFC] tag. If for some reason you want to
> > resend this, feel free to do so, but otherwise consider this applied.
> 
> Thanks!
> 
> The patch is short, it's understandable and it fixes the problem.
> I prefere to apply this patch as is.
> 
> I see some inconsistency in drivers/i2c/i2c.c.
> We have i2c_master_send() and i2c_write_reg() functions.
> These functions are intended to make similar work but they are
> written in very different style. I suppose that we can rewrite
> i2c_master_send() (e.g. drop FIXME) and make i2c_write_reg()
> work on top of i2c_master_send(). Any comments?

Rewrite i2c_master_send()? Do you mean rewrite i2c_write_reg() instead?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] commands: i2c_write: enable raw write to address
  2018-02-13  8:19     ` Sascha Hauer
@ 2018-02-14  6:19       ` Antony Pavlov
  2018-02-16  7:53         ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: Antony Pavlov @ 2018-02-14  6:19 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Tue, 13 Feb 2018 09:19:43 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Fri, Feb 09, 2018 at 02:22:24PM +0300, Antony Pavlov wrote:
> > On Fri, 9 Feb 2018 09:36:36 +0100
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > Hi Antony,
> > > 
> > > On Thu, Feb 08, 2018 at 10:48:56AM +0300, Antony Pavlov wrote:
> > > > Sometimes for communication with a simple I2C devices
> > > > (e.g. PCF8574 or TM1650) it's necessary to send only
> > > > one data byte into the I2C device.
> > > > Current i2c_write command makes this impossible because
> > > > you can't just pass 'device address' and 'register number'
> > > > (or 'device address' and 'one data byte') to the command.
> > > > You always have to pass all three parameters:
> > > > 'device address', 'register number' and 'data'.
> > > > 
> > > > This commit fixes the problem.
> > > > 
> > > > Sample usage:
> > > > 
> > > >   barebox@barebox sandbox:/ i2c_write -a 0x24 0x01
> > > > 
> > > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > > ---
> > > >  commands/i2c.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > Looks good to me.
> > > 
> > > I applied this one despite the [RFC] tag. If for some reason you want to
> > > resend this, feel free to do so, but otherwise consider this applied.
> > 
> > Thanks!
> > 
> > The patch is short, it's understandable and it fixes the problem.
> > I prefere to apply this patch as is.
> > 
> > I see some inconsistency in drivers/i2c/i2c.c.
> > We have i2c_master_send() and i2c_write_reg() functions.
> > These functions are intended to make similar work but they are
> > written in very different style. I suppose that we can rewrite
> > i2c_master_send() (e.g. drop FIXME) and make i2c_write_reg()
> > work on top of i2c_master_send(). Any comments?
> 
> Rewrite i2c_master_send()? Do you mean rewrite i2c_write_reg() instead?

You are right, of course I propose rewrite i2c_write_reg() (there is no
FIXME comment inside i2c_master_send()).

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] commands: i2c_write: enable raw write to address
  2018-02-14  6:19       ` Antony Pavlov
@ 2018-02-16  7:53         ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2018-02-16  7:53 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

On Wed, Feb 14, 2018 at 09:19:06AM +0300, Antony Pavlov wrote:
> On Tue, 13 Feb 2018 09:19:43 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > On Fri, Feb 09, 2018 at 02:22:24PM +0300, Antony Pavlov wrote:
> > > On Fri, 9 Feb 2018 09:36:36 +0100
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > 
> > > > Hi Antony,
> > > > 
> > > > On Thu, Feb 08, 2018 at 10:48:56AM +0300, Antony Pavlov wrote:
> > > > > Sometimes for communication with a simple I2C devices
> > > > > (e.g. PCF8574 or TM1650) it's necessary to send only
> > > > > one data byte into the I2C device.
> > > > > Current i2c_write command makes this impossible because
> > > > > you can't just pass 'device address' and 'register number'
> > > > > (or 'device address' and 'one data byte') to the command.
> > > > > You always have to pass all three parameters:
> > > > > 'device address', 'register number' and 'data'.
> > > > > 
> > > > > This commit fixes the problem.
> > > > > 
> > > > > Sample usage:
> > > > > 
> > > > >   barebox@barebox sandbox:/ i2c_write -a 0x24 0x01
> > > > > 
> > > > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > > > ---
> > > > >  commands/i2c.c | 8 ++++++--
> > > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > Looks good to me.
> > > > 
> > > > I applied this one despite the [RFC] tag. If for some reason you want to
> > > > resend this, feel free to do so, but otherwise consider this applied.
> > > 
> > > Thanks!
> > > 
> > > The patch is short, it's understandable and it fixes the problem.
> > > I prefere to apply this patch as is.
> > > 
> > > I see some inconsistency in drivers/i2c/i2c.c.
> > > We have i2c_master_send() and i2c_write_reg() functions.
> > > These functions are intended to make similar work but they are
> > > written in very different style. I suppose that we can rewrite
> > > i2c_master_send() (e.g. drop FIXME) and make i2c_write_reg()
> > > work on top of i2c_master_send(). Any comments?
> > 
> > Rewrite i2c_master_send()? Do you mean rewrite i2c_write_reg() instead?
> 
> You are right, of course I propose rewrite i2c_write_reg() (there is no
> FIXME comment inside i2c_master_send()).

Ok, that should work I think. Feel free to propose a patch.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-02-16  7:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-08  7:48 [RFC] commands: i2c_write: enable raw write to address Antony Pavlov
2018-02-09  8:36 ` Sascha Hauer
2018-02-09 11:22   ` Antony Pavlov
2018-02-13  8:19     ` Sascha Hauer
2018-02-14  6:19       ` Antony Pavlov
2018-02-16  7:53         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox