Hi Mario, On 10 May 2016 at 01:51, Mario Six <mario....@gdsys.cc> wrote: > This patch implements the open-drain setting feature for the MPC85XX > GPIO controller. > > Signed-off-by: Mario Six <mario....@gdsys.cc> > --- > > v3: > - Added missing commit message > - Fixed white space issues in function headers > > --- > drivers/gpio/Kconfig | 6 +++--- > drivers/gpio/mpc85xx_gpio.c | 19 +++++++++++++++++++ > 2 files changed, 22 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass <s...@chromium.org> But please see below. > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 068ee63..b250622 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -162,9 +162,9 @@ config MPC85XX_GPIO > configurable to match the actual GPIO count of the SoC (e.g. the > 32/32/23 banks of the P1022 SoC). > > - The standard functions of input/output mode, and output value > setting > - are supported; the open-drain capability of the controller is not > - supported yet. > + Aside from the standard functions of input/output mode, and output > + value setting, the open-drain feature, which can configure > individual > + GPIOs to work as open-drain outputs, is supported. > > The driver has been tested on MPC85XX, but it is likely that other > PowerQUICC III devices will work as well. > diff --git a/drivers/gpio/mpc85xx_gpio.c b/drivers/gpio/mpc85xx_gpio.c > index acf0414..dc6193c 100644 > --- a/drivers/gpio/mpc85xx_gpio.c > +++ b/drivers/gpio/mpc85xx_gpio.c > @@ -73,6 +73,25 @@ static inline void mpc85xx_gpio_set_high(struct ccsr_gpio > *base, > setbits_be32(&base->gpdir, gpios); > } > > +static inline int mpc85xx_gpio_open_drain_val(struct ccsr_gpio *base, > + unsigned int mask) > +{ > + /* Read the requested values */ > + return in_be32(&base->gpodr) & mask; > +} > + > +static inline void mpc85xx_gpio_open_drain_on(struct ccsr_gpio *base, > + unsigned int gpios) > +{ > + setbits_be32(&base->gpodr, gpios); Why gpios? This would normally be 'offset', indicating that it is the GPIO offset within the bank. Also the code seems odd - don't you need to convert the value into a mask? > +} > + > +static inline void mpc85xx_gpio_open_drain_off(struct ccsr_gpio *base, > + unsigned int gpios) > +{ > + clrbits_be32(&base->gpodr, gpios); > +} > + > static int mpc85xx_gpio_direction_input(struct udevice *dev, unsigned int > gpio) > { > struct mpc85xx_gpio_data *data = dev_get_priv(dev); > -- > 2.7.0.GIT > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot