Hi Mario,
On 24 May 2016 at 01:20, Mario Six <mario....@gdsys.cc> wrote:
> Hi Simon,
>
> On Mon, May 23, 2016 at 5:42 PM, Simon Glass <s...@chromium.org> wrote:
>> Hi Mario,
>>
>> On 23 May 2016 at 01:08, Mario Six <mario....@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....@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?
Yes that's fine.
>
> 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?
Yes please.
>
>>> +
>>> /* 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
