| Message ID | 20180713210920.3648-2-wsa+renesas@sang-engineering.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: recovery: fix GPIO usage for recovery | expand |
On 14/07/2018 05:09, Wolfram Sang wrote: > I2C is open drain, so set up the GPIO accordingly. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > drivers/i2c/busses/i2c-designware-master.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c > index fc7c255c80af..a546db80f53e 100644 > --- a/drivers/i2c/busses/i2c-designware-master.c > +++ b/drivers/i2c/busses/i2c-designware-master.c > @@ -665,7 +665,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev) > struct gpio_desc *gpio; > int r; > > - gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH); > + gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN); > if (IS_ERR(gpio)) { > r = PTR_ERR(gpio); > if (r == -ENOENT || r == -ENOSYS) > G'day Wolfram, This was intentional. The gpio we use to drive the i2c line is implemented in an FPGA and signals a buffer attached to the GPIO to drive scl OPEN drain. The GPIO is output only. The gpio setup can still specify the the GPIO be allocated OPEN drain if someone wishes to use a "smarter" gpio. So while the scl is open drain, there may be hardware in between that isn't. What would the correct way be to deal with that now?
Hi Phil, > > - gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH); > > + gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN); > > if (IS_ERR(gpio)) { > > r = PTR_ERR(gpio); > > if (r == -ENOENT || r == -ENOSYS) > > > > This was intentional. The gpio we use to drive the i2c line is implemented in an > FPGA and signals a buffer attached to the GPIO to drive scl OPEN drain. The GPIO is output > only. So, it is not possible to read SCL status then? Hmm, currently a working get_scl is required... > The gpio setup can still specify the the GPIO be allocated OPEN drain if someone wishes > to use a "smarter" gpio. > > So while the scl is open drain, there may be hardware in between that isn't. > What would the correct way be to deal with that now? Well, I don't know much about this IP core and how/where it is used. I just wonder what happens if another user comes along using an open-drain GPIO. Is that possible? I assume it is the same with SDA? Non open-drain? Output only? Regards, Wolfram
On 17/07/2018 17:09, Wolfram Sang wrote: > Hi Phil, > >>> - gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH); >>> + gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN); >>> if (IS_ERR(gpio)) { >>> r = PTR_ERR(gpio); >>> if (r == -ENOENT || r == -ENOSYS) >>> >> >> This was intentional. The gpio we use to drive the i2c line is implemented in an >> FPGA and signals a buffer attached to the GPIO to drive scl OPEN drain. The GPIO is output >> only. > > So, it is not possible to read SCL status then? Hmm, currently a working > get_scl is required... > >> The gpio setup can still specify the the GPIO be allocated OPEN drain if someone wishes >> to use a "smarter" gpio. >> >> So while the scl is open drain, there may be hardware in between that isn't. >> What would the correct way be to deal with that now? > > Well, I don't know much about this IP core and how/where it is used. I > just wonder what happens if another user comes along using an > open-drain GPIO. Is that possible? > > I assume it is the same with SDA? Non open-drain? Output only? > Just had a closer look at how it's setup here. Maybe the following helps. The designware core is routed thru the fpga fabric. Which provides and SCL SDA output enable pin. Recovery gpio are provided by a FPGA gpio IO core. This core has a fixed output and fixed input. Here's the relevant bit on how it's all combined. PWR_SDA_a / PWR_SCL_a are the signals to the outside world. All the other signals are internal I2c0_Dat_s <= PWR_SDA_a; I2c0_Clk_s <= PWR_SCL_a; PWR_SDA_a <= '0' when (I2c0_Dat_Oe_s = '1') else 'Z'; This bit of logic combines the i2c core and gpios. PWR_SCL_a <= '0' when ((I2c0_Clk_Oe_s = '1') or (PWR_SCL_rec_s = '0')) else 'Z'; , pio_io_in_port(1) => PWR_SCL_a , pio_io_in_port(2) => PWR_SDA_a , pio_io_out_port(1) => PWR_SCL_rec_s pio_io_out_port port is the fixed config for output pio_io_in_port is the fixed config for input The gpio input / outputs exist in the same ip core. PWR_SCL_rec_s is the recovery clock gpio signal. It needs to be driven high / low. There's no concept of HiZ internally in the FPGA. If there was some kinda of OpenDrain gpio driver that modelled a FET driven by a push pull GPIO I guess it could be made to work.
Hi Phil, > > So, it is not possible to read SCL status then? Hmm, currently a working > > get_scl is required... ... > > Well, I don't know much about this IP core and how/where it is used. I > > just wonder what happens if another user comes along using an > > open-drain GPIO. Is that possible? > > > > I assume it is the same with SDA? Non open-drain? Output only? > > > > Just had a closer look at how it's setup here. > Maybe the following helps. Thanks for the detailed explanation. I am just afraid it is a litle too detailed for me. I am not sure if I can read it correctly: When you read the SCL/SDA GPIO, does it return the true state of the SCL/SDA line or does it just reflect the value it was set to output? > There's no concept of HiZ internally in the FPGA. Which probably means SDA is to be treated the same as SCL -> push/pull. > If there was some kinda of OpenDrain gpio driver that modelled a FET > driven by a push pull GPIO I guess it could be made to work. Still, that sounds quite unlikely to me, so we can for now assume that all designware users will have push/pull? Disclaimer: I have zero experience with this core, I don't know how hard it is to modify or which versions are out there. Thanks for your help, Wolfram
On 24/07/2018 15:26, Wolfram Sang wrote: > Hi Phil, > >>> So, it is not possible to read SCL status then? Hmm, currently a working >>> get_scl is required... > ... >>> Well, I don't know much about this IP core and how/where it is used. I >>> just wonder what happens if another user comes along using an >>> open-drain GPIO. Is that possible? >>> >>> I assume it is the same with SDA? Non open-drain? Output only? >>> >> >> Just had a closer look at how it's setup here. >> Maybe the following helps. > > Thanks for the detailed explanation. I am just afraid it is a litle too > detailed for me. I am not sure if I can read it correctly: > > When you read the SCL/SDA GPIO, does it return the true state of the > SCL/SDA line or does it just reflect the value it was set to output? Yes it returns the true state of the output pin. I admit it's a bit odd from the classic GPIO point of view. > >> There's no concept of HiZ internally in the FPGA. > > Which probably means SDA is to be treated the same as SCL -> push/pull. Yes. They're both driven push/pull, but the try state of the line is available. > >> If there was some kinda of OpenDrain gpio driver that modelled a FET >> driven by a push pull GPIO I guess it could be made to work. > > Still, that sounds quite unlikely to me, so we can for now assume that > all designware users will have push/pull? I know of one other doing the same thing with the core in the Altera SocFPGA platform. As they put me on to this solution for doing the recovery when the i2c was routed thru the SOC's fpga. In other hard configurations they may have a 'proper' GPIO available that needs to be OpenDrain. > > Disclaimer: I have zero experience with this core, I don't know how hard > it is to modify or which versions are out there. >
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index fc7c255c80af..a546db80f53e 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -665,7 +665,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev) struct gpio_desc *gpio; int r; - gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH); + gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN); if (IS_ERR(gpio)) { r = PTR_ERR(gpio); if (r == -ENOENT || r == -ENOSYS)
I2C is open drain, so set up the GPIO accordingly. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- drivers/i2c/busses/i2c-designware-master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)