Re: [PATCH linux v1 0/4] Seven segment display support

[Rob Lippert <rlippert@google.com>]

On Wed, Dec 14, 2016 at 6:35 AM, Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Tue, Dec 13, 2016 at 11:55:00PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
>
> Jaghu,
>
> Bringing the discussion back onto just the OpenBMC list.
>
> Thank you for getting this out to the broader Linux community to solicit
> feedback.  As you see, even if things look good to some of the kernel
> devs here from a "is this generally how the kernel does things" and "is
> this code generally of quality to be acceptable upstream" there is a lot
> of history and design decisions on something as large as the Linux
> kernel for any of us to all keep track of.
>
> When we are adding support for a new device of a type already supported,
> like an i2c-eeprom or a thermal sensor, it is fairly easy to assume
> we're going to go through the upstream community.  When we are adding
> something new, like this 7-seg display, there is more likely to be
> resistance.  Maybe Joel can weigh in on, going forward, how we get early
> feedback so we don't spend a lot of time working on code that will never
> be accepted.
>
> I read through the current responses and the referenced email chain from
> 2013 with a similar driver and there seems to be two major points in the
> feedback.
>
> 1) The chip you are driving the 7-seg display with is already supported
> as a SPI-attached GPIO controller.
>
>     It does look like that driver plus SPI_BITBANG might allow us to
>     control the GPIOs on that device as a normal GPIO controller.  Is
>     that something you would like to try out and see if you can get
>     working?  Hopefully that would reduce your code to just the
>     translation from character to GPIO settings.

This was my original hope but Jagha tried it and it doesn't work :(

The serial GPIO interface is slightly different wrt the buffer chips
that we use vs the ones that are needed to work with the serial GPIO
driver (namely the lack of a "load" signal) and there is no way to
make it work as far as I can tell.

>
>
> 2) The community isn't interested in a one-off driver like this.
>
>     The thread from 2013 raised points about how a 7-segment display
>     driver isn't generic enough to support "X-segment displays".  One
>     of the authors mentioned how some displays have a decimal place that
>     can also be lit.  This lead Greg K-H to the decision that it would
>     best be handled from userspace.
>
>     I believe Google has a desire to get post codes from the BMC U-boot and
>     kernel, hence why you tried to put it into the kernel?  Are there
>     additional reasons for wanting to put it in the kernel?
>
>     I see three possible solutions, and there may be more:
>
>     a) We use SPI_BITBANG and the GPIO driver for this device (which we
>     probably should do in either case) and use device tree "hogging"
>     of the GPIOs to set the initial kernel post code.  This will cause
>     the kernel to light that display correctly as it is initializing.
>     We can then write a userspace application to manipulate the GPIOs
>     after that to change the display as appropriate.
>
>     b) We go back to the LKML with some more background on why we would
>     like this in-kernel, as in something unique to the BMC requirements
>     that needs the support in-kernel, and see if that makes them more
>     receptive.  I'm willing to help you work on the message here if you
>     would like.

The other part of this driver that hasn't been written yet is to take
bytes written to LPC port 80h (from the hostboot/etc. firmware) and
echo them to the 7seg display.

That would be easier to do purely in the kernel, although it should
also be possible to do it via a userspace app I guess with a lot more
work...

>
>
>     c) We see how willing Joel is to keep this as an out-of-tree driver.
>     It seems to use pretty generic and not-likely-to-change kernel APIs
>     so I don't see this as a large maintenance burden.
>
>     I suspect we would tend to prefer (a) but I don't know if that will
>     still allow it to meet Google's needs.
>
> Feel free to catch me on IRC today if you'd like to have an active
> discussion on this to get things moving faster.
>
> --
> Patrick Williams
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc
>