----- "Prafulla Wadaskar" <prafu...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
