How to add GPIO outputs to the PXA2xx MFP configuration?

[Paul Parsons]

--- On Fri, 30/3/12, Paul Parsons <lost.distance at yahoo.com> wrote:
> --- On Fri, 30/3/12, Igor Grinberg
> <grinberg at compulab.co.il>
> wrote:
> > On 03/30/12 18:52, Paul Parsons
> > wrote:
> > > --- On Fri, 30/3/12, Igor Grinberg <grinberg at compulab.co.il>
> > wrote:
> > >> On 03/30/12 18:06, Paul Parsons wrote:
> > >>> --- On Fri, 30/3/12, Igor Grinberg <grinberg at compulab.co.il>
> > >> wrote:
> > >>>> On 03/28/12 20:12, Paul Parsons
> > >>>> wrote:
> > >>>>> On PXA2xx platforms, the MFP API
> > (described in
> > >>>> Documentation/arm/pxa/mfp.txt)
> > >>>>> provides values for the
> following:
> > >>>>>
> > >>>>> 1. GPIO inputs (e.g.
> GPIO105_GPIO).
> > >>>>> 2. Alternate function inputs
> (e.g.
> > >> GPIO105_CIF_DD_1).
> > >>>>> 3. Alternate function outputs
> (e.g.
> > >>>> GPIO105_KP_MKOUT_2).
> > >>>>>
> > >>>>> It does not provide values for
> GPIO
> > outputs
> > >> (i.e. AF0
> > >>>> outputs).
> > >>>>>
> > >>>>> One cannot use the macro used by
> the
> > MFP API
> > >> internally
> > >>>> - MFP_CFG_OUT() - to
> > >>>>> define new GPIO output values,
> since
> > that macro
> > >> is
> > >>>> forbidden in platform code.
> > >>>>>
> > >>>>> Without the ability to add GPIO
> outputs
> > to the
> > >> MFP
> > >>>> configuration, it is not
> > >>>>> possible to drive GPIO outputs
> high
> > during
> > >> sleep mode.
> > >>>>>
> > >>>>> This would be useful, for example,
> on
> > the
> > >> hx4700
> > >>>> platform, where driving the
> > >>>>> infrared powerdown GPIO 105 high
> during
> > sleep
> > >> mode
> > >>>> would save some mA.
> > >>>>>
> > >>>>> So my question is: what method
> should
> > one use
> > >> to add
> > >>>> GPIO outputs to the MFP
> > >>>>> configuration?
> > >>>>>
> > >>>>> One possible method, namely
> manually
> > defining
> > >> values in
> > >>>> the platform code:
> > >>>>>
> > >>>>>      MFP_PIN_GPIO105 |
> > >> MFP_AF0 |
> > >>>> MFP_DIR_OUT | MFP_LPM_DRIVE_HIGH,
> > >>>>
> > >>>> Have you tried:
> > >>>> GPIO105_GPIO | MFP_LPM_DRIVE_HIGH,
> > >>>> ?
> > >>>>
> > >>>> This way it works on other boards.
> > >>>
> > >>> Hello Igor,
> > >>>
> > >>> GPIO105_GPIO expands to a GPIO input -
> > >> MFP_CFG_IN(GPIO105, AF0) -
> > >>> not an output.
> > >>
> > >> Yeah, I know... but read in mfp-pxa2xx.h,
> lines
> > 6-20:
> > >> /*
> > >>  * the following MFP_xxx bit definitions in
> > mfp.h are
> > >> re-used for pxa2xx:
> > >>  *
> > >>  *  MFP_PIN(x)
> > >>  *  MFP_AFx
> > >>  *  MFP_LPM_DRIVE_{LOW, HIGH}
> > >>  *  MFP_LPM_EDGE_x
> > >>  *
> > >>  * other MFP_x bit definitions will be
> > ignored
> > >>  *
> > >>  * and adds the below two bits specifically
> > for pxa2xx:
> > >>  *
> > >>  * bit     23 -
> > Input/Output (PXA2xx
> > >> specific)
> > >>  * bit     24 - Wakeup
> > Enable(PXA2xx
> > >> specific)
> > >>  */
> > >>
> > >> This means, that there is no difference
> between:
> > MFP_CFG_IN
> > >> and MFP_CFG_OUT for PXA2xx.
> > > 
> > > The mfp-pxa2xx.c code says otherwise.
> > 
> > That's just great... outdated documentation...
> > 
> > > It uses the MFP direction bit
> > > to set the GPDR registers on entering sleep mode:
> > > 
> > >    353    static int
> > pxa2xx_mfp_suspend(void)
> > >    354    {
> > >    355       
> > int i;
> > >         ...
> > >    368       
> > for (i = 0; i <= gpio_to_bank(pxa_last_gpio); i++)
> {
> > >    369    
> > >         ...
> > >    375       
> >     GPDR(i * 32) = gpdr_lpm[i];
> > >    376       
> > }
> > >    377       
> > return 0;
> > >    378    }
> > > 
> > > gpdr_lpm[] is initialized at startup to the
> current
> > GPDR values, and
> > > thereafter is updated by the value of the
> MFP_DIR_OUT
> > bit in calls to
> > > __mfp_config_gpio().
> > 
> > static int __mfp_config_gpio(unsigned gpio, unsigned
> long
> > c)
> > {
> >     unsigned long gafr, mask =
> > GPIO_bit(gpio);
> >     int bank = gpio_to_bank(gpio);
> >     int uorl = !!(gpio & 0x10); /*
> > GAFRx_U or GAFRx_L ? */
> >     int shft = (gpio & 0xf) << 1;
> >     int fn = MFP_AF(c);
> >     int is_out = (c & MFP_DIR_OUT) ? 1 :
> > 0;
> > 
> > [...]
> > 
> >     /* alternate function and direction at
> > low power mode */
> >     switch (c & MFP_LPM_STATE_MASK) {
> >     case MFP_LPM_DRIVE_HIGH:
> >         PGSR(bank) |= mask;
> >         is_out = 1;
> >         break;
> >     case MFP_LPM_DRIVE_LOW:
> >         PGSR(bank) &=
> > ~mask;
> >         is_out = 1;
> >         break;
> > 
> > And in the lines above, it gets overridden by
> > MFP_LPM_DRIVE_*,
> > or am I missing something?
> 
> No, that looks to be the case: the LPM states DRIVE_HIGH and
> DRIVE_LOW
> override the sleep mode direction. I completely missed
> that.
> 
> > [...]
> > 
> >     if (is_out ^
> > gpio_desc[gpio].dir_inverted)
> >         gpdr_lpm[bank] |=
> > mask;
> >     else
> >         gpdr_lpm[bank] &=
> > ~mask;
> > 
> > 
> > So, I think,
> > GPIO105_GPIO | MFP_LPM_DRIVE_HIGH,
> > as I've proposed in my first email, should still work
> for
> > you.
> 
> Yes it should. Thanks.

However MFP_LPM_KEEP_OUTPUT does not do likewise, so for example:

    GPIO72_GPIO | MFP_LPM_KEEP_OUTPUT,

will set GPIO72 as an input in sleep mode. hmm...