[PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

[Dave Stevenson]

On 21 March 2017 at 20:58, Michael Zoran <mzoran at crowfest.net> wrote:
> On Tue, 2017-03-21 at 23:30 +0300, Dan Carpenter wrote:
>> I've read Stefan's review comments and intervened where I didn't
>> understand them.  The recent ones are very specific and reasonable...
>>
>> Just send a v2.  I don't get what the deal is...
>
> Partly because Eric and Stefan have publically annouced all over the
> who knows where that they don't want this in staging.  They say it
> needs to go to the general tree.  So I'm not 100% sure a V2 makes sense
> when people aren't even agreeing if this is the correct place for this
> to be.
>
> I did not write the driver myself, I've simply been burdened myself
> that VC4 doesn't really work that good on the RPI 3 so I was hoping it
> would unblock people after finding it on the web.
>
> If people want a V2, then they are going to have to be very specific
> about what needs to change and exactly how it to change. Not just say
> they don't like it.
>
> If it's suddenly so important, why can't someone else add in their
> corrections after my Signed-off-by: and add their own Signed-off-by:.
> Or better yet submit the original driver themselves.

Michael: I know the original request to upstream this came from Phil /
myself, and thank you for your efforts. However if you want to offload
this then I will find some time to go through a couple of revisions.
It would be Tuesday or Wednesday next week all being well.
This is a pretty trivial driver so there's only limited scope for me
messing it up (hopefully), and the lack of HDMI HPD is a stumbling
block for Pi3/CM3.

Can I get a couple of answers from others as to preferences first?

Sorry Eric, I wasn't aware you had previously sent a patch
implementing any of this otherwise I would have taken it as a base.
I'd seen a comment that you were thinking about it but that was all.
So which patch is preferred as the base? Eric's or mine?

Eric's patch had the driver in the main tree vs Michael putting it in
staging. Where do we really want this going?
Linus Walleij as one of the GPIO maintainers had reviewed Eric's patch
and not totally dismissed it, so are we better off picking up on the
comments from both review threads and aiming to merge directly to
drivers/gpio?

Linus' review comments preferred hard coding "ngpios" and
"firmware-gpio-offset" in the driver. Eric and Michael both pushed
them into DT. Stephan made a comment of setting rpi->gc.base
explicitly in the driver. I know I was focussed on the task of getting
it working, hence the hardcoded gc.base=128 and then using gc.base as
the firmware offset.
I can see cases for numerous options:
1) Any expanders controlled through the firmware will start with an
offset of 128. Within the firmware GPIOs 0-127 map onto the standard
GPIO block, but why would you bother to use this driver when you can
go direct and avoid an IPC overhead? Hard coding that offset to 128
seems reasonable.
2) The mailbox service call being used doesn't restrict control to
just being GPIOs on the expander, so ngpios isn't necessarily always
8. The same expander chip is used on the Compute Module 3 but with
only 2 GPIOs used (HDMI HPD and EMMC_EN). I can't predict the future,
but alternative expanders may get added at a later date that have a
different number of GPIOs. Linux wouldn't need to know the details
beyond ngpios.
3) rpi->gc.base being 128 matches the firmware references and pin
definitions. There is nothing stopping you having different reference
numbers under Linux to the firmware, but is there a useful case for
that?
Consensus on best approach? DT properties (potentially optional), or
omit from DT and hardcode?

Module name? rpi_gpio, brcmexp_gpio, or something else?
Following on. DT compatible string - "raspberrypi,firmware-gpio",
"brcm,bcm2835-expgpio", or something else.
(Can I get a bigger can to put the worms back in?!)

Required patch list?
1) device tree binding documentation. Required as we need to document
at least the firmware link.
2) update "raspberrypi-firmware.h" for the new enums (Can we combine
this with the driver? Eric had, and Linus was happy with an ack from
Eric as the maintainer there)
3) the driver itself.
4) DT updates for bcm2837-rpi-3-b.dts (is there a CM3 dts file
somewhere?). I was naughty in the Foundation tree and merged the
driver with the DT changes to use it for HDMI HPD and power LED
control, but those will need to be split out. Done as a follow up
patch once the driver is merged?


The Foundation tree can be changed to match this pretty swiftly to
avoid having two variants kicking around for long. My version may be
out in the wild, but nothing should be referring to it directly by
name so most changes can be hidden behind DT.

Comments welcome.

  Dave

PS I've dropped Greg KH and devel at driverdev.osuosl.org partly as we're
discussing next steps, and partly as my SMTP server keeps rejecting my
message as spam probably due to too many recipients.