[U-Boot] [PATCH v4 4/4] dm: test: Add GPIO open drain tests

[Mario Six]

Hi Simon,

On Mon, May 23, 2016 at 5:42 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mario,
>
> On 23 May 2016 at 01:08, Mario Six <mario.six at gdsys.cc> wrote:
>> Add some tests for the new open drain setting feature of the GPIO
>> uclass, and extend the capabilities of the sandbox GPIO driver
>> accordingly.
>>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>> ---
>>
>> v4:
>> Patch added
>>
>> ---
>>  drivers/gpio/sandbox.c | 35 +++++++++++++++++++++++++++++++++++
>>  test/dm/gpio.c         |  7 +++++++
>>  2 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
>> index a9b1efc..f6435a0 100644
>> --- a/drivers/gpio/sandbox.c
>> +++ b/drivers/gpio/sandbox.c
>> @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>  /* Flags for each GPIO */
>>  #define GPIOF_OUTPUT   (1 << 0)        /* Currently set as an output */
>>  #define GPIOF_HIGH     (1 << 1)        /* Currently set high */
>> +#define GPIOF_ODR      (1 << 2)        /* Currently set to open drain mode */
>>
>>  struct gpio_state {
>>         const char *label;      /* label given by requester */
>> @@ -70,6 +71,16 @@ int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value)
>>         return set_gpio_flag(dev, offset, GPIOF_HIGH, value);
>>  }
>>
>> +int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset)
>> +{
>> +       return get_gpio_flag(dev, offset, GPIOF_ODR);
>> +}
>> +
>> +int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value)
>> +{
>> +       return set_gpio_flag(dev, offset, GPIOF_ODR, value);
>> +}
>> +
>>  int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset)
>>  {
>>         return get_gpio_flag(dev, offset, GPIOF_OUTPUT);
>> @@ -124,6 +135,28 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value)
>>         return sandbox_gpio_set_value(dev, offset, value);
>>  }
>>
>> +/* read GPIO ODR value of port 'offset' */
>> +static int sb_gpio_get_open_drain(struct udevice *dev, unsigned offset)
>> +{
>> +       debug("%s: offset:%u\n", __func__, offset);
>> +
>> +       return sandbox_gpio_get_open_drain(dev, offset);
>> +}
>> +
>> +/* write GPIO ODR value to port 'offset' */
>> +static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value)
>> +{
>> +       debug("%s: offset:%u, value = %d\n", __func__, offset, value);
>> +
>> +       if (!sandbox_gpio_get_direction(dev, offset)) {
>> +               printf("sandbox_gpio: error: set_open_drain on input gpio %u\n",
>> +                      offset);
>> +               return -1;
>> +       }
>> +
>> +       return sandbox_gpio_set_open_drain(dev, offset, value);
>> +}
>> +
>>  static int sb_gpio_get_function(struct udevice *dev, unsigned offset)
>>  {
>>         if (get_gpio_flag(dev, offset, GPIOF_OUTPUT))
>> @@ -154,6 +187,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = {
>>         .direction_output       = sb_gpio_direction_output,
>>         .get_value              = sb_gpio_get_value,
>>         .set_value              = sb_gpio_set_value,
>> +       .get_open_drain         = sb_gpio_get_open_drain,
>> +       .set_open_drain         = sb_gpio_set_open_drain,
>>         .get_function           = sb_gpio_get_function,
>>         .xlate                  = sb_gpio_xlate,
>>  };
>> diff --git a/test/dm/gpio.c b/test/dm/gpio.c
>> index 727db18..4779b5a 100644
>> --- a/test/dm/gpio.c
>> +++ b/test/dm/gpio.c
>> @@ -75,6 +75,13 @@ static int dm_test_gpio(struct unit_test_state *uts)
>>         ut_assertok(ops->set_value(dev, offset, 1));
>>         ut_asserteq(1, ops->get_value(dev, offset));
>>
>> +       /* Make it an open drain output, and reset it */
>> +       ut_asserteq(0, ops->get_open_drain(dev, offset));
>> +       ut_assertok(ops->set_open_drain(dev, offset, 1));
>> +       ut_asserteq(1, ops->get_open_drain(dev, offset));
>> +       ut_assertok(ops->set_open_drain(dev, offset, 0));
>> +       ut_asserteq(0, ops->get_open_drain(dev, offset));
>
> Instead of reading back from the driver, you should call
> sandbox_gpio_get_open_drain() here. The idea is to set the value
> through the API, then check (via the back door sandbox functions) that
> it happened.
>
Ah, OK. So, something like

ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 1));
ut_asserteq(1, sandbox_gpio_get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 0));
ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset));

Will be enough?

Also, while adding sandbox_gpio_{get,set}_open_drain to
arch/sandbox/include/asm/gpio.h, I noticed that some parameter names in the
documentation seem a bit off:

/**
 * Return the simulated value of a GPIO (used only in sandbox test code)
 *
 * @param gp    GPIO number
 * @return -1 on error, 0 if GPIO is low, >0 if high
 */
int sandbox_gpio_get_value(struct udevice *dev, unsigned int offset);

Should I fix that in a separate patch?

>> +
>>         /* Make it an input */
>>         ut_assertok(ops->direction_input(dev, offset));
>>         ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
>> --
>> 2.7.0.GIT
>>
>
> Regards,
> Simon

Best regards,
Mario