<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 30, 2020 at 12:14 PM Christian Mauderer <<a href="mailto:oss@c-mauderer.de">oss@c-mauderer.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Gedare,<br>
<br>
thanks for the review.<br>
<br>
On 30/11/2020 18:43, Gedare Bloom wrote:<br>
> <br>
> <br>
> On Mon, Nov 30, 2020 at 7:14 AM Christian Mauderer <br>
> <<a href="mailto:christian.mauderer@embedded-brains.de" target="_blank">christian.mauderer@embedded-brains.de</a> <br>
> <mailto:<a href="mailto:christian.mauderer@embedded-brains.de" target="_blank">christian.mauderer@embedded-brains.de</a>>> wrote:<br>
> <br>
> ---<br>
> shell/general_commands.rst | 200 +++++++++++++++++++++++++++++++++++++<br>
> 1 file changed, 200 insertions(+)<br>
> <br>
> diff --git a/shell/general_commands.rst b/shell/general_commands.rst<br>
> index c74ae45..a6f7e18 100644<br>
> --- a/shell/general_commands.rst<br>
> +++ b/shell/general_commands.rst<br>
> @@ -44,6 +44,14 @@ The RTEMS shell has the following general commands:<br>
> <br>
> - rtc_ - RTC driver configuration<br>
> <br>
> +- i2cdetect_ - detect I2C devices<br>
> +<br>
> +- i2cget_ - get data from an EEPROM like I2C device<br>
> +<br>
> +- i2cset_ - write data to an EEPROM like I2C device<br>
> +<br>
> +- spi_ - read and write simple data to an SPI bus<br>
> +<br>
> - exit_ - alias for logoff command<br>
> <br>
> Commands<br>
> @@ -1179,6 +1187,198 @@ CONFIGURATION:<br>
> <br>
> \clearpage<br>
> <br>
> +.. _i2cdetect:<br>
> +<br>
> +i2cdetect - detect I2C devices<br>
> +------------------------------<br>
> +.. index:: i2cdetect<br>
> +<br>
> +SYNOPSYS:<br>
> + .. code-block:: shell<br>
> +<br>
> + i2cdetect <I2C_BUS><br>
> +<br>
> +.. index:: CONFIGURE_SHELL_NO_COMMAND_I2CDETECT<br>
> +.. index:: CONFIGURE_SHELL_COMMAND_I2CDETECT<br>
> +<br>
> +DESCRIPTION:<br>
> + Tries to detect I2C devices connected to the I2C bus. To do<br>
> that, write<br>
> + requests with the length of 0 are used.<br>
> +<br>
> + WARNING: This might confuse some I2C devices, so please use it<br>
> only if you<br>
> + know what you are doing.<br>
> +<br>
> <br>
> What happens for devices that don't know how to respond? is it any kind <br>
> of undefined behavior?<br>
<br>
That depends on the devices. Basically everything can happen. For example:<br>
<br>
- device is just not detected<br>
- I2C bus hangs<br>
- some random undefined behavior<br>
<br>
The command uses the same method that is used by the Linux i2cdetect <br>
command and therefore I added a similar warning:<br>
<br>
<a href="https://linux.die.net/man/8/i2cdetect" rel="noreferrer" target="_blank">https://linux.die.net/man/8/i2cdetect</a><br>
<br>
> <br>
> + The command supports a ``-h`` option to get usage details.<br>
> +<br>
> + The command works only with I2C bus drivers that use the<br>
> Linux-Style API.<br>
> +<br>
> +EXAMPLES:<br>
> + The following is an example where two I2C devices are detected.<br>
> One on 0x1a<br>
> + and one on 0x1f:<br>
> +<br>
> + .. code-block:: shell<br>
> +<br>
> + SHLL [/] # i2cdetect /dev/i2c1<br>
> + x0 x1 x2 x3 x4 x5 x6 x7 x8 x9 xA xB xC xD xE xF<br>
> + 0x -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + 1x -- -- -- -- -- -- -- -- -- -- 1a -- -- -- -- 1f<br>
> + 2x -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + 3x -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + 4x -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + 5x -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + 6x -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + 7x -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --<br>
> + SHLL [/] #<br>
> +<br>
> +CONFIGURATION:<br>
> + This command is included in the default shell command set. <br>
> When building a<br>
> <br>
> If this is unsafe to use when you don't know what you're doing, then <br>
> maybe it should not be available without explicitly turning it on? <br>
> (Safe Defaults)<br>
<br>
I can do that if you want. It's more or less a diagnostic command (just <br>
like the others here). But note that we have a lot of "unsafe if you <br>
don't know what you are doing" commands. Like dd, mkrfs, mmove, fdisk, <br>
and even exit can crash some systems ...<br>
<br>
Aren't nearly all shell commands at least a bit unsafe? And we don't <br>
have a lot of commands that have to be explicitly turned on if <br>
CONFIGURE_SHELL_COMMANDS_ALL is already set. I found only networking <br>
commands.<br>
<br></blockquote><div>That seems accurate, you can go ahead with the approach you took then. It seems to be mostly consistent with the state of practice. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> + custom command set, define<br>
> ``CONFIGURE_SHELL_COMMAND_I2CDETECT`` to have<br>
> + this command included.<br>
> +<br>
> + This command can be excluded from the shell command set by defining<br>
> + ``CONFIGURE_SHELL_NO_COMMAND_I2CDETECT`` when all shell<br>
> commands have been<br>
> + configured.<br>
> +<br>
> +.. raw:: latex<br>
> +<br>
> + \clearpage<br>
> +<br>
> +.. _i2cget:<br>
> +<br>
> +i2cget - get data from an EEPROM like I2C device<br>
> +------------------------------------------------<br>
> +.. index:: i2cget<br>
> +<br>
> +SYNOPSYS:<br>
> + .. code-block:: shell<br>
> +<br>
> + i2cget <I2C_BUS> <CHIP-ADDRESS> <DATA-ADDRESS> [<NR-BYTES>]<br>
> +<br>
> +.. index:: CONFIGURE_SHELL_NO_COMMAND_I2CGET<br>
> +.. index:: CONFIGURE_SHELL_COMMAND_I2CGET<br>
> +<br>
> +DESCRIPTION:<br>
> + Get one or multiple bytes from an EEPROM like I2C device. If<br>
> you read<br>
> + multiple bytes (<NR-BYTES> given and > 1) the read will be done<br>
> in one<br>
> <br>
> <br>
> Default is 1 byte? Please clarify<br>
<br>
OK.<br>
<br>
> <br>
> + single request. An auto incrementing register pointer is assumed.<br>
> +<br>
> + The command supports a ``-h`` option to get usage details.<br>
> +<br>
> + The command works only with I2C bus drivers that use the<br>
> Linux-Style API.<br>
> <br>
> <br>
> Would it make more sense to include all these under an <br>
> CONFIGURE_SHELL_COMMAND_I2CLINUX or something?<br>
<br>
I'm quite indifferent regarding that. If you prefer it, I can collect them.<br>
<br>
Beneath CONFIGURE_SHELL_COMMANDS_ALL and <br>
CONFIGURE_SHELL_COMMANDS_ALL_NETWORKING there don't seem to be a lot of <br>
groups. So I didn't start a new one ...<br>
<br></blockquote><div>ok, let's not make it more complex than needed. I guess the main advantage would be code size.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> I don't use the shell though so take everything I say with a bit of <br>
> skepticism.<br>
<br>
I'll wait for some more feedback.<br>
<br>
> <br>
> +<br>
> +EXAMPLES:<br>
> + The following is an example how to read a one byte register at<br>
> 0xd from the<br>
> + I2C device at 0x1f:<br>
> +<br>
> + .. code-block:: shell<br>
> +<br>
> + SHLL [/] # i2cget /dev/i2c1 0x1f 0x0d<br>
> <br>
> <br>
> Again, my shell ignorance: do the addresses need to be specified as Hex <br>
> in 0x format?<br>
<br>
I wrote the commands in a way that they should accept hex (0x10), <br>
decimal (16) or octal (020). I can add this as a comment.<br>
<br>
> <br>
> + 0xc7<br>
> + SHLL [/] #<br>
> +<br>
> +CONFIGURATION:<br>
> + This command is included in the default shell command set. <br>
> When building a<br>
> + custom command set, define ``CONFIGURE_SHELL_COMMAND_I2CGET``<br>
> to have this<br>
> + command included.<br>
> +<br>
> + This command can be excluded from the shell command set by defining<br>
> + ``CONFIGURE_SHELL_NO_COMMAND_I2CGET`` when all shell commands<br>
> have been<br>
> + configured.<br>
> +<br>
> +.. raw:: latex<br>
> +<br>
> + \clearpage<br>
> +<br>
> +.. _i2cset:<br>
> +<br>
> +i2cset - write data to an EEPROM like I2C device<br>
> +------------------------------------------------<br>
> +.. index:: i2cset<br>
> +<br>
> +SYNOPSYS:<br>
> + .. code-block:: shell<br>
> +<br>
> + i2cset <I2C_BUS> <CHIP-ADDRESS> <DATA-ADDRESS> <VALUE><br>
> [<VALUE> [...]]<br>
> +<br>
> +.. index:: CONFIGURE_SHELL_NO_COMMAND_I2CSET<br>
> +.. index:: CONFIGURE_SHELL_COMMAND_I2CSET<br>
> +<br>
> +DESCRIPTION:<br>
> + Write one or multiple bytes to an EEPROM like I2C device. If<br>
> you write<br>
> + multiple bytes (multiple <VALUE> given) the write will be done<br>
> in one single<br>
> <br>
> <br>
> Same for the values, do they need to be in 0x format, or is there <br>
> already documentation of what formats are accepted?<br>
<br>
No there is no other documentation. Maybe I should add a bit. Do you <br>
think it would be better to add it here or to the "-h" output?<br>
<br></blockquote><div>I think here is good.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> + request. An auto incrementing register pointer is assumed.<br>
> +<br>
> + The command supports a ``-h`` option to get usage details.<br>
> +<br>
> + The command works only with I2C bus drivers that use the<br>
> Linux-Style API.<br>
> +<br>
> +EXAMPLES:<br>
> + The following is an example how to write one byte of 0x00 to<br>
> the register at<br>
> + 0x11 of the I2C device at 0x1f:<br>
> +<br>
> + .. code-block:: shell<br>
> +<br>
> + SHLL [/] # i2cset /dev/i2c1 0x1f 0x11 0x00<br>
> + SHLL [/] #<br>
> +<br>
> +CONFIGURATION:<br>
> + This command is included in the default shell command set. <br>
> When building a<br>
> + custom command set, define ``CONFIGURE_SHELL_COMMAND_I2CSET``<br>
> to have this<br>
> + command included.<br>
> +<br>
> + This command can be excluded from the shell command set by defining<br>
> + ``CONFIGURE_SHELL_NO_COMMAND_I2CSET`` when all shell commands<br>
> have been<br>
> + configured.<br>
> +<br>
> +.. raw:: latex<br>
> +<br>
> + \clearpage<br>
> +<br>
> +.. _spi:<br>
> +<br>
> +spi - read and write simple data to an SPI bus<br>
> +----------------------------------------------<br>
> +.. index:: spi<br>
> +<br>
> +SYNOPSYS:<br>
> + .. code-block:: shell<br>
> +<br>
> + spi [-loh] [-c <cs>] [-s <speed>] [-m <mode>] <SPI_BUS> xx<br>
> [xx [..]]<br>
> +<br>
> +.. index:: CONFIGURE_SHELL_NO_COMMAND_SPI<br>
> +.. index:: CONFIGURE_SHELL_COMMAND_SPI<br>
> +<br>
> +DESCRIPTION:<br>
> + Write data to an SPI bus and read the responses.<br>
> +<br>
> + The command supports a ``-h`` option to get usage details.<br>
> +<br>
> + The command works only with SPI bus drivers that use the<br>
> Linux-Style API.<br>
> +<br>
> +EXAMPLES:<br>
> + The following is an example how to write multiple bytes (0x42<br>
> 0x43 0x44) to<br>
> + the bus. The response is three times 0x00 in this case because<br>
> the bus has<br>
> + been left open. Chip select 1 will be used.<br>
> +<br>
> + .. code-block:: shell<br>
> +<br>
> + SHLL [/] # spi /dev/spi1 -c 1 42 0x43 44<br>
> + received: 00 00 00<br>
> + SHLL [/] #<br>
> +<br>
> <br>
> <br>
> Provide an example to read?<br>
<br>
Maybe I should have added some values other than 0x00 to the received <br>
line. SPI can only do both at once (at least for the normal drivers).<br>
<br>
> Why not unify the i2c command the same way as spi? Just curious if there <br>
> is some rationale.<br>
<br>
Rationale is that I used Linux I2C tools as a rough guide for the <br>
syntax. For SPI I didn't have a template.<br>
<br>
Beneath that I2C does have explicit read or write modes. So I would have <br>
to set the direction with a parameter if I unified them. SPI on the <br>
other hand does always receive and transmit at the same time. So it <br>
wouldn't be useful to split it up.<br>
<br></blockquote><div><br></div><div>I see, nevermind about the unification question then. Thanks, just clear up some of the default behavior and go ahead.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Best regards<br>
<br>
Christian<br>
<br>
> <br>
> +CONFIGURATION:<br>
> + This command is included in the default shell command set. <br>
> When building a<br>
> + custom command set, define ``CONFIGURE_SHELL_COMMAND_SPI`` to<br>
> have this<br>
> + command included.<br>
> +<br>
> + This command can be excluded from the shell command set by defining<br>
> + ``CONFIGURE_SHELL_NO_COMMAND_SPI`` when all shell commands have<br>
> been<br>
> + configured.<br>
> +<br>
> +.. raw:: latex<br>
> +<br>
> + \clearpage<br>
> +<br>
> .. _exit:<br>
> <br>
> exit - exit the shell<br>
> -- <br>
> 2.26.2<br>
> <br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a> <mailto:<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a>><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> <<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a>><br>
> <br>
> <br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> <br>
</blockquote></div></div>