[U-Boot] [PATCH 3/4] gpio: xilinx: Not read output values via regs
Michal Simek
michal.simek at xilinx.com
Thu Aug 2 12:56:25 UTC 2018
On 1.8.2018 20:36, Stefan Herbrechtsmeier wrote:
> Am 30.07.2018 um 21:34 schrieb Stefan Herbrechtsmeier:
>> Am 30.07.2018 um 16:10 schrieb Michal Simek:
>>> On 30.7.2018 15:32, Stefan Herbrechtsmeier wrote:
>>>> Am 30.07.2018 um 14:40 schrieb Michal Simek:
>>>>> On 27.7.2018 10:41, Stefan Herbrechtsmeier wrote:
>>>>>> Am 27.07.2018 um 09:05 schrieb Michal Simek:
>>>>>>> On 26.7.2018 21:46, Stefan Herbrechtsmeier wrote:
>>>>>>>> Am 26.07.2018 um 10:41 schrieb Michal Simek:
>>>>>>>>> On 25.7.2018 20:21, Stefan Herbrechtsmeier wrote:
>>>>>>>>>> Am 25.07.2018 um 08:39 schrieb Michal Simek:
>>>>>>>>>>> On 24.7.2018 21:56, Stefan Herbrechtsmeier wrote:
>>>>>>>>>>>> Am 24.07.2018 um 12:31 schrieb Michal Simek:
>>>>>>>>>>>>> On 23.7.2018 20:42, Stefan Herbrechtsmeier wrote:
>>>>>>>>>>>>>> Am 23.07.2018 um 13:43 schrieb Michal Simek:
>>>>>>>>>>>>>>> Reading registers for finding out output value is not
>>>>>>>>>>>>>>> working
>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>> input value is read instead in case of tristate.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Reported-by: Stefan Herbrechtsmeier
>>>>>>>>>>>>>>> <stefan at herbrechtsmeier.net>
>>>>>>>>>>>>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> drivers/gpio/xilinx_gpio.c | 38
>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++-----
>>>>>>>>>>>>>>> 1 file changed, 33 insertions(+), 5 deletions(-)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git a/drivers/gpio/xilinx_gpio.c
>>>>>>>>>>>>>>> b/drivers/gpio/xilinx_gpio.c
>>>>>>>>>>>>>>> index 4da9ae114d87..9d3e9379d0e5 100644
>>>>>>>>>>>>>>> --- a/drivers/gpio/xilinx_gpio.c
>>>>>>>>>>>>>>> +++ b/drivers/gpio/xilinx_gpio.c
>>>>>>>>>>>> [snip]
>>>>>>>>>>>>
>>>>>>>>>>>>>>> + priv->output_val[bank] = val;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> return val;
>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>> @@ -441,6 +449,7 @@ static int
>>>>>>>>>>>>>>> xilinx_gpio_get_function(struct
>>>>>>>>>>>>>>> udevice *dev, unsigned offset)
>>>>>>>>>>>>>>> static int xilinx_gpio_get_value(struct udevice
>>>>>>>>>>>>>>> *dev,
>>>>>>>>>>>>>>> unsigned
>>>>>>>>>>>>>>> offset)
>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>> struct xilinx_gpio_platdata *platdata =
>>>>>>>>>>>>>>> dev_get_platdata(dev);
>>>>>>>>>>>>>>> + struct xilinx_gpio_privdata *priv = dev_get_priv(dev);
>>>>>>>>>>>>>>> int val, ret;
>>>>>>>>>>>>>>> u32 bank, pin;
>>>>>>>>>>>>>>> @@ -451,7 +460,14 @@ static int
>>>>>>>>>>>>>>> xilinx_gpio_get_value(struct
>>>>>>>>>>>>>>> udevice
>>>>>>>>>>>>>>> *dev, unsigned offset)
>>>>>>>>>>>>>>> debug("%s: regs: %lx, gpio: %x, bank %x, pin
>>>>>>>>>>>>>>> %x\n",
>>>>>>>>>>>>>>> __func__,
>>>>>>>>>>>>>>> (ulong)platdata->regs, offset, bank, pin);
>>>>>>>>>>>>>>> - val = readl(&platdata->regs->gpiodata + bank
>>>>>>>>>>>>>>> * 2);
>>>>>>>>>>>>>>> + if (xilinx_gpio_get_function(dev, offset) ==
>>>>>>>>>>>>>>> GPIOF_INPUT) {
>>>>>>>>>>>>>>> + debug("%s: Read input value from reg\n", __func__);
>>>>>>>>>>>>>>> + val = readl(&platdata->regs->gpiodata + bank * 2);
>>>>>>>>>>>>>>> + } else {
>>>>>>>>>>>>>>> + debug("%s: Read saved output value\n", __func__);
>>>>>>>>>>>>>>> + val = priv->output_val[bank];
>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>> Why you don't always read the data register? This doesn't
>>>>>>>>>>>>>> work for
>>>>>>>>>>>>>> three
>>>>>>>>>>>>>> state outputs.
>>>>>>>>>>>>> In three state register every bit/pin is 0 - output, 1 input.
>>>>>>>>>>>>> It means else part is output and I read saved value in
>>>>>>>>>>>>> priv->output_val.
>>>>>>>>>>>>> If pin is setup as INPUT then I need read data reg to find out
>>>>>>>>>>>>> input
>>>>>>>>>>>>> value.
>>>>>>>>>>>>> Maybe you are commenting something else but please let me
>>>>>>>>>>>>> know if
>>>>>>>>>>>>> there
>>>>>>>>>>>>> is any other bug.
>>>>>>>>>>>> What happen if I have an open drain output. Even if the gpio
>>>>>>>>>>>> output
>>>>>>>>>>>> is 1
>>>>>>>>>>>> the input could read a 0. You driver will always return the
>>>>>>>>>>>> output
>>>>>>>>>>>> value
>>>>>>>>>>>> and not the real input value. According to the picture in
>>>>>>>>>>>> documentation
>>>>>>>>>>>> and my tests a data register write writes the output registers
>>>>>>>>>>>> and a
>>>>>>>>>>>> data register read reads the input registers.
>>>>>>>>>>>>
>>>>>>>>>>>> Why should the driver return the desired state (output
>>>>>>>>>>>> register)
>>>>>>>>>>>> and not
>>>>>>>>>>>> the real state (input register)?
>>>>>>>>>>> First of all thanks for description.
>>>>>>>>>>>
>>>>>>>>>>> I have another example where you have output only and you can't
>>>>>>>>>>> read
>>>>>>>>>>> input because there is no wire.
>>>>>>>>>> Does you mean the all outputs configuration? Does this removes
>>>>>>>>>> the
>>>>>>>>>> "gpio_io_i" signal from the IP?
>>>>>>>>> I am not sure what synthesis is doing with that unused IP pins
>>>>>>>>> but I
>>>>>>>>> would consider as a bug if this is automatically connected
>>>>>>>>> together.
>>>>>>>> I mean does the IP generator removes the gpio_io_i signal
>>>>>>>> because it
>>>>>>>> isn't needed? If the IP generator creates the gpio_io_i signal I
>>>>>>>> would
>>>>>>>> expect that you can't leave it unconnected as this would lead to
>>>>>>>> undefined values.
>>>>>>> Normally when you know that you have output only there is no no
>>>>>>> gpio_io_i or tristate signal. The same for input only.
>>>>>> And in this case the device tree flags "xlnx,all-inputs" or
>>>>>> "xlnx,all-outputs" should be set.
>>>>> yes.
>>>>>
>>>>>>>>> And
>>>>>>>>> also wasting a logic if there is unused part.
>>>>>>>>> But in Vivado you should be able to setup output pins to and input
>>>>>>>>> pins
>>>>>>>>> separately. There are In/Out/Tristate.
>>>>>>>>> If you don't want to deal with external pin you can connect them
>>>>>>>>> inside PL.
>>>>>>>> This isn't my point. I mean that if you have an gpio_io_i signal
>>>>>>>> you
>>>>>>>> have to connected it to a signal. You could connect it to the
>>>>>>>> output of
>>>>>>>> an IO, to the gpio_io_o signal or to fixed value (0 or 1). If you
>>>>>>>> connect it to a fixed value (0 or 1) you get this value if you read
>>>>>>>> the
>>>>>>>> status of this GPIO.
>>>>>>> Not a HW to tell you what's vivado is going to do with that. But you
>>>>>>> can
>>>>>>> select that ouput/input only where these signals are out.
>>>>>> You mean the all-inputs or all-outputs case.
>>>>> yes.
>>>>>
>>>>>>>>>>> Regarding open drain output.
>>>>>>>>>>>
>>>>>>>>>>> Also this is what it is written in manual:
>>>>>>>>>>> "AXI GPIO Data Register Description"
>>>>>>>>>>> "For each I/O bit programmed as output:
>>>>>>>>>>> • R: Reads to these bits always return zeros"
>>>>>>>>>> This must be a mistake in the documentation. I could read the
>>>>>>>>>> input
>>>>>>>>>> value.
>>>>>>>>>>
>>>>>>>>>> The old driver and the Linux driver always read the register.
>>>>>>>>> In old u-boot driver I see
>>>>>>>>> 179 if (gpio_get_direction(gpio) ==
>>>>>>>>> GPIO_DIRECTION_OUT)
>>>>>>>>> 180 val = gpio_get_output_value(gpio);
>>>>>>>>> 181 else
>>>>>>>>> 182 val = gpio_get_input_value(gpio);
>>>>>>>>>
>>>>>>>>> gpio_get_output_value() reads it from gpiodata_store for output
>>>>>>>>> gpio_get_input_value() reads the reg.
>>>>>>>> Sorry you are right. I mixed it with the zynq driver.
>>>>>>> ok.
>>>>>>>
>>>>>>>
>>>>>>>>> In Linux you are right it is read from reg in both cases.
>>>>>>>>>
>>>>>>>>>> Additionally the documentation states that a write to an input
>>>>>>>>>> doesn't
>>>>>>>>>> work:
>>>>>>>>>> AXI GPIO Data Register.
>>>>>>>>>> For each I/O bit programmed as input:
>>>>>>>>>> • R: Reads value on the input pin.
>>>>>>>>>> • W: No effect.
>>>>>>>>>>
>>>>>>>>>> However the Linux driver use the common sequence and sets
>>>>>>>>>> first the
>>>>>>>>>> data
>>>>>>>>>> register and afterwards the direction register.
>>>>>>>>> Possible that driver has pretty long history and I don't think
>>>>>>>>> it was
>>>>>>>>> tested for all cases.
>>>>>>>> I test the IP again and it works like expected and the
>>>>>>>> documentation is
>>>>>>>> wrong. You could always write the output register and read the
>>>>>>>> input
>>>>>>>> register. This means you could write the output first and
>>>>>>>> afterwards
>>>>>>>> enable the output via the tri-state register.
>>>>>>> ok.
>>>>>> Maybe the set_direction function should be adapted to avoid glitches
>>>>>> because it first enabled the output with the old value.
>>>>> No problem with this change at all.
>>>>>
>>>>>>>>>>> If you want to support this configuration then you would have to
>>>>>>>>>>> change
>>>>>>>>>>> pin to input and read it and revert it back. And all of this
>>>>>>>>>>> should be
>>>>>>>>>>> done based on flag.
>>>>>>>>>>> There is macro GPIO_OPEN_DRAIN which could be specified via DT
>>>>>>>>>>> binding.
>>>>>>>>>>> Unfortunately this is for consumer not for generic listing. I
>>>>>>>>>>> can't
>>>>>>>>>>> also
>>>>>>>>>>> see it in gpio_desc flags to be able to handle it in the driver.
>>>>>>>>>>> That's why I have doubts if this is supported by any u-boot gpio
>>>>>>>>>>> driver
>>>>>>>>>>> but I can be wrong.
>>>>>>>>>> I think the GPIO_OPEN_DRAIN is used to not set the output to 1.
>>>>>>>>>> In our
>>>>>>>>>> case the open drain is transparent. The output has only two
>>>>>>>>>> states
>>>>>>>>>> High-Z and 0. The input value of the FPGA output block is always
>>>>>>>>>> 0 and
>>>>>>>>>> the tri-state input is connected to the output register of the
>>>>>>>>>> GPIO
>>>>>>>>>> controller. The output of the FPGA input block is connected to
>>>>>>>>>> the
>>>>>>>>>> input
>>>>>>>>>> register of the GPIO controller.
>>>>>>>>> I just found out in connection to second thread we have together
>>>>>>>>> that in
>>>>>>>>> gpio-uclass there are dm_gpio_set/get_open_drain functions but
>>>>>>>>> they are
>>>>>>>>> not wired anywhere.
>>>>>>>>> I think in your case when these two functions are implemented you
>>>>>>>>> will
>>>>>>>>> get this functionality which you are asking about.
>>>>>>>> No. This function is to control a special register.
>>>>>>> I am not quite sure if this is really about special register or
>>>>>>> can be
>>>>>>> also used in our case.
>>>>>> In my case the open drain is transparent for the controller. The
>>>>>> output
>>>>>> replace a 1 by a High-Z. This only means that you don't know the real
>>>>>> value on the wire.
>>>>> please look below.
>>>>>
>>>>>>>> Beside the open drain. What happens if the output is stuck to a
>>>>>>>> fixed
>>>>>>>> signal. This could be detected if you read the input register.
>>>>>>> And again if that wires are there. Not covering the case where
>>>>>>> you have
>>>>>>> output only without any input signal at all.
>>>>>> This can be detect by the all-outputs flag.
>>>>> please look below.
>>>>>
>>>>>>>>> My guess is that gpio command should be extended to call these two
>>>>>>>>> functions. When set_open_drain is called special flag for pin is
>>>>>>>>> setup
>>>>>>>>> and for this pin get_status will always read data reg in our
>>>>>>>>> case and
>>>>>>>>> you get your values.
>>>>>>>> The main quest is which value should the get function return for an
>>>>>>>> output. The value from the input register which represents the real
>>>>>>>> value or the value from the output register which represents the
>>>>>>>> desired
>>>>>>>> value. Because of the "Warning: value of pin is still %d" inside
>>>>>>>> the
>>>>>>>> gpio cmd I would expect the real value. This is also implemented by
>>>>>>>> most
>>>>>>>> drivers. Only tegra, kw and rcar distinguish between input and
>>>>>>>> output.
>>>>>>>> Thereby the rcar explicitly mention a bug:
>>>>>>>>
>>>>>>>> Testing on r8a7790 shows that INDT does not show correct pin state
>>>>>>>> when
>>>>>>>> configured as output, so use OUTDT in case of output pins.
>>>>>>>>
>>>>>>>> A test of the Xilinx GPIO controller shows that a read always
>>>>>>>> returns
>>>>>>>> the value of the gpio_io_i signal.
>>>>>>> What about to use this logic?
>>>>>>>
>>>>>>> if (platdata->bank_input[bank]) // input only
>>>>>>> val = readl(&platdata->regs->gpiodata + bank * 2);
>>>>>>> else if (platdata->bank_output[bank])) // output only
>>>>>>> val = priv->output_val[bank];
>>>>>> Okay
>>>>>>
>>>>>>> else { // tristate because it is not output or input only.
>>>>>>> state = xilinx_gpio_get_function(dev, offset)
>>>>>>> if (state == GPIOF_INPUT) // if input now - just read it
>>>>>>> val = readl(&platdata->regs->gpiodata + bank * 2);
>>>>>>> else { // if output - change it to input
>>>>>>> tmp1 = readl(&platdata->regs->gpiodir + bank * 2);
>>>>>>> tmp = tmp1 | (1 << pin);
>>>>>>> writel(tmp, &platdata->regs->gpiodir + bank * 2);
>>>>>>>
>>>>>>> // read value
>>>>>>> val = readl(&platdata->regs->gpiodata + bank * 2);
>>>>>>>
>>>>>>> // revert it back
>>>>>>> writel(tmp1, &platdata->regs->gpiodir + bank *
>>>>>>> 2);
>>>>>>> }
>>>>>>> }
>>>>>> This will generate a glitch on the wire.
>>>>>>
>>>>>> Why don't we simple read the value? The hardware support this and we
>>>>>> have already handle the all-output case:
>>>>>>
>>>>>> if (platdata->bank_output[bank]))
>>>>>> val = priv->output_val[bank];
>>>>>> else
>>>>>> val = readl(&platdata->regs->gpiodata + bank * 2);
>>>>>>
>>>>>> If you don't trust my test could you please ask your hardware co
>>>>>> works
>>>>>> why the data register behavior should depend on the tri-state
>>>>>> register.
>>>>> I am looking for IP owner but as far as I see it was created 9
>>>>> years ago
>>>>> that's why it will take some time.
>>>>> Anyway I have created hw design on zcu102 which has output on led and
>>>>> input on dip and output value is kept in data reg that's why your
>>>>> snippet above can be used.
>>>> Do you use a signal port for both or two separated ports?
>>> I expect single - yes the same pin 0 for output led and input dip.
>>
>> The same pin or port?
>>
>>>> Have you connect the gpio_io_i and gpio_io_o signals of the outputs?
>>> It is really pin 0 io_o to one fpga pin and pin0 io_i to another fpga
>>> pin.
>>
>> To make sure I understand you correct: The gpio_io_o[0] is connected
>> to a LED and gpio_io_i[0] is connected to a switch?
>>
>> Have you connect the gpio_io_t[0] signal?
>>
>>>> What happens if you connect the gpio_io_i signals to zero for the
>>>> output
>>>> to save resources?
>>>> What happens if you disable the power supply for the LED bank?
>>> didn't try that.
>>>
>>>>> Documentation is wrong in this
>>>>> "For each I/O bit programmed as output:
>>>>> • R: Reads to these bits always return zeros"
>>>>>
>>>>> I have sent 4 patches which we discussed that's why please reviewed
>>>>> them.
>>>>> I have one more patch in queue which is keeping output_val for certain
>>>>> bank but not sure if make sense to apply it because as above when
>>>>> direction is setup to output you can read desired output value from
>>>>> gpiodata reg.
>>>> This is only true if the gpio_io_i and gpio_io_o signals are connected
>>>> together or the input and output signals have always the same value.
>>>> Otherwise you read the input signal and write it into the output value.
>>> That's what I see on zcu102 without any connection between io_i io_o.
>>> There is incorrect behavior when gpio toggle command is called and input
>>> value is 1. Because the first gpio toggle cmd reads input value first
>>> (because code is doing that) and then setting up output with opposite
>>> value.
>>>
>>>
>>>> In our use case we have two gpio controllers (for two processors) which
>>>> are connected to one io port. Thereby only the output is multiplexed
>>>> between the two controllers and the input is always connected to the
>>>> io.
>>>> This means both controller always read the input value of the io but
>>>> only one of them controls the output of the io. Your set function reads
>>>> the input value of the io (maybe controlled by the other controller)
>>>> and
>>>> thereby overwrite the old output value of the controller.
>>>>
>>>>> I have tested it on gpio output only led case and gpiodata
>>>>> reg contain correct value.
>>>> Have you test outputs with differences between output value and input
>>>> value?
>>> I believe so.
>>>
>>>>> Anyway if I miss any patch in connection to this please send a
>>>>> patch and
>>>>> let's look at that use case.
>>>> Okay
>>> TBH it is starting to be a little bit confusing.
>>
>> Me too.
>>
>> It looks like we have observe different behavior. I will redo my test
>> in the next days.
>>
>> It would be so much easier if we could check the IP's HDL.
>>
>> The hardware have three physical registers (gpio_io_o, gpio_io_i,
>> gpio_io_t) per pin and two bus registers (GPIOx_DATA, GPIOx_TRI) per
>> port.
>>
>> -> GPIOx_TRI
>> -> Read
>> -> gpio_io_t
>> -> Write
>> -> gpio_io_t
>>
>> -> GPIOx_DATA
>> -> Read
>> -> gpio_io_i (my observation)
>> -> (gpio_io_t == 1) ? gpio_io_i : gpio_io_o (your
>> observation ?)
>> -> Write
>> -> gpio_io_o
>>
>>> What we should really do is to use standard boards and connection and
>>> describe that cases and expected behavior and changes in code which
>>> should happen to support this.
>>>
>>> We are talking about two functions.
>>> xilinx_gpio_set_value and xilinx_gpio_get_value.
>>>
>>> And let's take mainline driver with that 4 patches I have sent.
>>
>> Okay
>>
>>> Let's start with xilinx_gpio_get_value()
>>> Mainline is all the time reading value from gpiodata reg no matter if
>>> this is output/input only or io.
>>>
>>> Above is this sequence which you have written that it is right
>>> if (platdata->bank_output[bank]))
>>> val = priv->output_val[bank];
>>> else
>>> val = readl(&platdata->regs->gpiodata + bank * 2);
>>
>> Based on your observations the driver should work without this change
>> because the controller returns the output register value if the
>> tristate register is cleared. Hopefully this also happens if the
>> all_outputs flag is set.
>>
>>> I have took at look at gpio output only case
>>>
>>> ZynqMP> gpio toggle gpio at a00010000
>>> gpio: pin gpio at a00010000 (gpio 187) value is 1
>>> ZynqMP> gpio status -a gpio at a0001000
>>> Bank gpio at a0001000:
>>> gpio at a00010000: output: 1 [ ]
>>> gpio at a00010001: output: 0 [ ]
>>> gpio at a00010002: output: 0 [ ]
>>> gpio at a00010003: output: 0 [ ]
>>> gpio at a00010004: output: 0 [ ]
>>> gpio at a00010005: output: 0 [ ]
>>> gpio at a00010006: output: 0 [ ]
>>> gpio at a00010007: output: 0 [ ]
>>> ZynqMP> gpio toggle gpio at a00010004
>>> gpio: pin gpio at a00010004 (gpio 191) value is 1
>>> ZynqMP> gpio status -a gpio at a0001000
>>> Bank gpio at a0001000:
>>> gpio at a00010000: output: 1 [ ]
>>> gpio at a00010001: output: 0 [ ]
>>> gpio at a00010002: output: 0 [ ]
>>> gpio at a00010003: output: 0 [ ]
>>> gpio at a00010004: output: 1 [ ]
>>> gpio at a00010005: output: 0 [ ]
>>> gpio at a00010006: output: 0 [ ]
>>> gpio at a00010007: output: 0 [ ]
>>> ZynqMP> gpio toggle gpio at a00010006
>>> gpio: pin gpio at a00010006 (gpio 193) value is 1
>>> ZynqMP> gpio status -a gpio at a0001000
>>> Bank gpio at a0001000:
>>> gpio at a00010000: output: 1 [ ]
>>> gpio at a00010001: output: 0 [ ]
>>> gpio at a00010002: output: 0 [ ]
>>> gpio at a00010003: output: 0 [ ]
>>> gpio at a00010004: output: 1 [ ]
>>> gpio at a00010005: output: 0 [ ]
>>> gpio at a00010006: output: 1 [ ]
>>> gpio at a00010007: output: 0 [ ]
>>> ZynqMP> gpio toggle gpio at a00010004
>>> gpio: pin gpio at a00010004 (gpio 191) value is 0
>>> ZynqMP> gpio status -a gpio at a0001000
>>> Bank gpio at a0001000:
>>> gpio at a00010000: output: 1 [ ]
>>> gpio at a00010001: output: 0 [ ]
>>> gpio at a00010002: output: 0 [ ]
>>> gpio at a00010003: output: 0 [ ]
>>> gpio at a00010004: output: 0 [ ]
>>> gpio at a00010005: output: 0 [ ]
>>> gpio at a00010006: output: 1 [ ]
>>> gpio at a00010007: output: 0 [ ]
>>> ZynqMP> gpio toggle gpio at a00010007
>>> gpio: pin gpio at a00010007 (gpio 194) value is 1
>>> ZynqMP> gpio status -a gpio at a0001000
>>> Bank gpio at a0001000:
>>> gpio at a00010000: output: 1 [ ]
>>> gpio at a00010001: output: 0 [ ]
>>> gpio at a00010002: output: 0 [ ]
>>> gpio at a00010003: output: 0 [ ]
>>> gpio at a00010004: output: 0 [ ]
>>> gpio at a00010005: output: 0 [ ]
>>> gpio at a00010006: output: 1 [ ]
>>> gpio at a00010007: output: 1 [ ]
>>>
>>> And for this case there is no need to take that value from any saved
>>> location.
>>> It means having
>>> val = readl(&platdata->regs->gpiodata + bank * 2);
>>> is enough based on my test.
>>> And for the rest as your example above this should be also fine.
>>
>> Yes. If your observations are correct it is impossible to read the
>> input state if the GPIO is configured as output and if my observations
>> are correct the read will return the input state independently of the
>> mode.
>>
>>> When we have agreement on this we can look at xilinx_gpio_set_value.
>>
>> The direct read is fine for me.
>>
>> Can you read the two switch states if you run the following commands:
>>
>> /* Enable LED */
>> ZynqMP> gpio set gpio at a00010000
>> /* Read switch state and keep LED on */
>> ZynqMP> gpio input gpio at a00010000
>> ZynqMP> gpio status -a gpio at a0001000
>> /* Manually change switch state and read again */
>> ZynqMP> gpio status -a gpio at a0001000
>> /* Disable LED */
>> ZynqMP> gpio clear gpio at a00010000
>> ZynqMP> gpio status -a gpio at a0001000
>> /* Manually change switch state and read again */
>> ZynqMP> gpio status -a gpio at a0001000
>>
>> Best regards
>> Stefan
>>
>
> I have test the GPIO controller by read and write the register via the
> md and mw command. The read *always* returns the status of the gpio_io_i
> signal independent of the gpio_io_t status. This means to support my
> version of the IP the driver needs to save the output value in a
> variable. This solution should work for your and my hardware. Could you
> repost your patch to add the dout-default and output_val or should I
> post a patch based on your patch?
Interesting. Anyway I have sent v2 I hope will all things we have discussed.
Thanks,
Michal
More information about the U-Boot
mailing list