----- "Prafulla Wadaskar" prafulla@marvell.com wrote:
You should define all GPIO register in a single struct And point them with base address offsets
I suggest you to add mvgpio.c instead of armada100_gpio.c This will be used in future by other Marvell SoCs.
GPIO registers might vary for other Marvell SoCs? How to deal with that?
+int gpio_request(int gp, const char *label) +{
- /*
* Assumes corresponding MFP is configured peoperly* for use as GPIO*/NAK, you should check here, respective MFP is being configured as GPIO, if not you should return error
I checked datasheet and for most GPIOs, AF1 is given as GPIO but for few its not, so adding a glue logic to check for specific GPIOs wont be a good idea. Thats the reason i thought its good to keep MFP out of this. Please give suggestions.
Consider now this is generic GPIO driver for marvell SOCs, define this macro in gpio.h
ok
- if (gp < ARMD_MAX_GPIO) {
switch (GPIO_TO_REG(gp)) {Some code comments are welcomed here.
case 0:writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr0);break;case 1:writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr1);break;case 2:writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr2);break;case 3:writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr3);break;default:return -EINVAL;}- } else {
return -EINVAL;- }
- return 0;
+}
+int gpio_direction_output(int gp, int value) +{
- struct armdgpio_registers *gpio_regs =
(struct armdgpio_registers *)ARMD1_GPIO_BASE;- if (gp < ARMD_MAX_GPIO) {
switch (GPIO_TO_REG(gp)) {case 0:writel(GPIO_TO_BIT(gp), &gpio_regs->gsdr0);Call gpio_set_value(gp, value) here which is defined below doing the same
thanks for suggestion.
panic("Invalid GPIO pin %u\n", gp);}- } else {
panic("Invalid GPIO pin %u\n", gp);Can you eliminate one panic line here?
Regards.. Prafulla . .
will do that... there is no return value from this function so i thought it wont be good to continue from this point incase designed direction is not set.
Regards, Ajay Bhargav