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