From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Thu, 2 Aug 2018 14:56:25 +0200 Subject: [U-Boot] [PATCH 3/4] gpio: xilinx: Not read output values via regs In-Reply-To: <8d318d2d-a58c-a327-52e7-068528c93c90@herbrechtsmeier.net> References: <61f5486be81d3a2e9170aa7c383de68b78b944cb.1532346215.git.michal.simek@xilinx.com> <36e1b314-74a5-04bc-8563-629c5ebeda22@xilinx.com> <76009864-c697-e216-a7ad-2ad8f728c0c1@herbrechtsmeier.net> <7891cd4a-3c00-720b-3e0d-539630b4b119@xilinx.com> <6d7886e9-289f-3048-f3f5-75db2f9b4d0a@herbrechtsmeier.net> <050995d0-2ae3-20b7-0a99-c63e3a7fa031@xilinx.com> <17f7640c-001a-8680-8cf7-ec98bf116c9f@herbrechtsmeier.net> <9c2933bf-190c-e471-a51e-106af88740d5@herbrechtsmeier.net> <82f9b81c-82dd-9719-e61f-d06a51248f64@xilinx.com> <7f7fd70e-7f06-5225-043b-4fc357b11da0@herbrechtsmeier.net> <2d78e24a-dfe7-eb0d-0477-ffe2afe0258a@xilinx.com> <8d318d2d-a58c-a327-52e7-068528c93c90@herbrechtsmeier.net> Message-ID: <9b89a2ac-3340-c3c2-f311-74211b846add@xilinx.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Signed-off-by: Michal Simek >>>>>>>>>>>>>>> --- >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>         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