How to handle GPIO differences between P8 and P9

[Xo Wang]

On Tue, Nov 8, 2016 at 1:13 PM, Mine <mine260309 at gmail.com> wrote:

> On Mon, Nov 7, 2016 at 3:53 PM, Xo Wang <xow at google.com> wrote:
> > On Mon, Nov 7, 2016 at 9:01 AM, Mine <mine260309 at gmail.com> wrote:
> >> On Mon, Nov 7, 2016 at 10:19 AM, Patrick Williams <patrick at stwcx.xyz>
> wrote:
> >>> On Mon, Nov 07, 2016 at 05:10:59PM +0800, Yi TZ Li wrote:
> >>>> I committed a patch: https://gerrit.openbmc-project.xyz/#/c/1019/
> >>>> to add a getSystemName() dbus method in org.obmc.managers.System
> interface.
> >>>> There are several places in skeleton requires fix for a specific
> system.
> >>>>
> >>>> Other way be add a compile flag in yocto when building Skeleton.
> >>>
> >>> We are really trying to avoid "if (machine_type == ...)" type code.
> This
> >>> becomes a gigantic problem when we are talking about supporting 3 dozen
> >>> machines.
> >>>
> >>> --
> >>> Patrick Williams
> >>
> >> Yup, I prefer not to write `if (machine_type == ...)` type code, nor
> >> to have a compile flag (it just becomes `#if xxx` or `#ifdef xxx`).
> >>
> >> The temp fix by not defining `IDBTN` will not only cause error log,
> >> but also prevent the code in `op-hostctl/control_host_obj.c` to work,
> >> because it will get error during opening the GPIO, and jump out,
> >> resulting in error: "GPIO sequence failed".
> >>
> >> If we are targeting for booting P9 up, I would suggest a temp fix by
> >> defining `IDBTN` for both Witherspoon and Romulus.
> >> It will not block power on, but may break a function related to LED.
> >>
> >> --
> >> BRs,
> >> Lei YU
> >> _______________________________________________
> >> openbmc mailing list
> >> openbmc at lists.ozlabs.org
> >> https://lists.ozlabs.org/listinfo/openbmc
> >
> > We had a similar issue in the op-pwrctl where the source hard-coded
> > the system net names. There I put in a short-term solution of adding a
> > function-specific abstraction over the net names into the System.py
> > config, removing the per-machine patches for name and polarity:
> > https://gerrit.openbmc-project.xyz/#/c/606/
> > https://gerrit.openbmc-project.xyz/#/c/689/
> >
> > It's not difficult to extend POWER_CONFIG for GPIO functions that only
> > exist on one machine, like so:
> > https://gerrit.openbmc-project.xyz/#/c/608/3
> >
> > With the abstraction taking place in the Python machine config, we
> > don't need per-machine dispatch in the user interface (op-pwrctl,
> > op-hostctl, op-pwr-button, etc).
> >
> > Does that work for you in the short term?
> >
> > cheers
> > xo
>
> Yup, that could be a solution. It's just the GPIOs are not for power
> control,
> so following the solution would probably introduce similar code to
> hostctl_gpio.h/c.
>
> Maybe I can combine the power_gpio and hostctl_gpio, so they share the same
> code?
>
> Something like:
>
> GPIO_CONFIG = {
>   'power_gpios': {
>     'latch_out' : 'BMC_UCD_LATCH_LE',
>     'power_good_in' : 'SYS_PWROK_BUFF',
>     'power_up_outs' : [
>         ('SOFTWARE_PGOOD', True),
>         ('BMC_POWER_UP', True),
>     ],
>     'reset_outs' : [
>     ],
>   },
>   'hostctl_gpios': {
>     'fsi_data' : 'FSI_DATA',
>     ...
>     'optionals' : [
>       ('BMC_THROTTLE', True),
>       ('IDBTN', False),
>     ],
>   }
> }
>
> What do you think?
>
> --
> BRs,
> Lei YU
>

Yes, that sounds reasonable to me. Maybe change the name of the PowerGpio
stuff to reflect your more general approach.

FYI something that I didn't implement was adding GPIO polarity support
into libopenbmc_intf/gpio. So, all the polarity (active high/low) logic
needs to be handled by the application, which is less than ideal.

cheers
xo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20161108/fe37f3f4/attachment-0001.html>