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

[Patrick Williams <patrick@stwcx.xyz>]

[-- Attachment #1: Type: text/plain, Size: 3468 bytes --]

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.

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.

    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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]