[RFC]: extend gpio and i2c #22
Conversation
Allow to use all GPIOs to be used as outputs if they are able to. Dissalow GPIO 12 to be used as output as we don't know it's function and the vendors library dissalow to use it. Pins 16 to 19 can only be used as output. Signed-off-by: Bernhard Guillon <Bernhard.Guillon@begu.org>
Check if the I2C driver already requested the I2C GPIOs. Disable the whole sector from bits 16 to 19 if I2C is enabled. Signed-off-by: Bernhard Guillon <Bernhard.Guillon@begu.org>
As the gpio driver command cannot disable individual GPIOs for the pin range 16-19 we need to grab them all. Otherwise writing to 16 or 17 will change the signals of the SDA and SCL lines. Signed-off-by: Bernhard Guillon <Bernhard.Guillon@begu.org>
| { | ||
| /* | ||
| * FIMXE: as this is a child of a mfd driver just return | ||
| * 1 for now :/ |
There was a problem hiding this comment.
You should do like the spi driver; be child of gpio.
| * Pin configuration. The I2C interfaces use 2 pins | ||
| * allocated to GPIOs. But we need to grab 4 pins | ||
| * to make sure that we don't mix signals. | ||
| */ |
There was a problem hiding this comment.
Maybe the gpio driver should use the new method if i2c isn't loaded/used and use the previous method if it is.
There was a problem hiding this comment.
I think I made a missleading comment here. We can still use the new method as it will still have the advantage to have more pins availabe without any interfering. The only problem are the last 4 GPIOs.
Do you prefer a check that the driver is not loaded over the is the line used in the gpio code? This would have the advantage that we might not need to change the i2c driver at all. But the disadvantage that the amount of exported GPIOs differs. And what should happen if one of the GPIO lines is already used? I'm not sure what's easier to understand for the user.
But it might be a good first solution to get this PR done.
| */ | ||
| static const u16 pin_can_output = 0b111111; | ||
| static const u32 pin_can_output = 0b11111110111111111111; | ||
| static const u32 pin_can_input = 0b1111111111111111; |
There was a problem hiding this comment.
Could you add some leading zeros to align the numbers so we can better see?
Hi,
I prepared the first proof of concept for the extension of the gpio interface #5 . Following the release early release often concept this PR is not in the shape to be directly merged.
What does this RFC tries to implement?
Allow to use all GPIOs and still have a working I2C driver.
Problems:
The main problem is that the chip command used for write outputs cannot independently enable the GPIOs from 16-19 as it is possible for all other GPIOs.
Also the bit 12 should not be allowed to be an output. As it might be the internal reset and is clearly masked out in any of the vendors libraries and other drivers. I this state we can leave it as input. Otherwise the scope of this PR would be to big and a distraction of the main problems.
Why is that bad?
If we initialize e.g. GPIO 16 and load the I2C driver with grabbing GPIO 18 and 19 ANY call to write_outputs might change the signal of the I2C pins. As it writes what ever is currently in its last written buffer.
How this RFC tries to solve that problem?
This PR tries to solve that problem by claiming all 4 GPIOs (16-19) mark 16,17 as reserverd 18 as SCL and 19 as SDA. At the gpio driver it adds a function to check if i2c is active which is trigger each time a output is added. This function currently uses the gpiochip_is_requested(chip, 18); and checks if the name matches SCL.
https://www.kernel.org/doc/html/latest/driver-api/gpio/index.html
This function is for use by GPIO controller drivers. The label can help with diagnostics, and knowing that the signal is used as a GPIO can help avoid accidentally multiplexing it to another controller.
If it detects that I2C is active the whole area (16-19) is disabled. This works pretty well.
Problems with this solution?
One problem is that I am unaware how to check if we got the right gpiochip as the i2c is a child of the mfd driver and the pdev->mfd_cell->platform_data is a null pointer.
Other solutions?
As both, the gpio and the i2c drivers are children of the mfd driver the could use syscon or a simple function to set and get the state of the i2c driver.
What do you think? Which direction should we further investigate? Also what are the plans for the spi driver, this is currently not a subchild of the mfd?
Thanks,
mr-nice