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

[Stefan Wahren]

> Dave Stevenson <dave.stevenson at raspberrypi.org> hat am 23. März 2017 um 23:07 geschrieben:
> 
> 
> Hi Stefan.
> 
> On 23 March 2017 at 19:45, Stefan Wahren <stefan.wahren at i2se.com> wrote:
> > Hi Dave,
> >
> >> Dave Stevenson <dave.stevenson at raspberrypi.org> hat am 23. März 2017 um 18:08 geschrieben:
> >>
> >> 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.
> >
> > i think there is a big misunderstanding. There is no standard GPIO block. The base in mainline pinctrl-bcm2835 is -1, which means Linux is free to choose the base.
> 
> My apologies if I'm misrepresenting you (and certainly for mispelling
> your name).

No problem

> I was referencing your comment archived at [1] in relation to Eric's
> patch implementing a driver for the expander.
> "i think it's better to assign rpi->gc.base explicit."
> Is that no longer your view?

This was a special case because devm_kzalloc() set the gc.base implicit to zero. So i suggest to set the base to an explicit value (no preferences which value).

As there are special DT properties like gpio-range-list [2] to set those parameters, i suggest to use them instead of reinvent them.

[2] - http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/gpio/gpio.txt

As i wrote before i suggest to make a RFC based on Michaels patch series but with all comments fixed and send them to the GPIO / DT maintainer. So we will have a good base for discussion and we can get feedback if we are going the right direction. If you are unsure about something, add a TODO comment or something else.

In case of a new driver i prefer to create the DT binding first, because it has a big influence on the driver.

> 
> My comment is that the VPU firmware code is using GPIO numbers 0-127
> for the peripheral that is generally termed the gpio block, even if
> Linux drives it through a pinctrl driver and is free to set the base
> to any value of its chosing.
> The mailbox service can be told to control the GPIOs the firmware
> considers to be 0-127 just as well as those on the expander starting
> at 128, however in my view there is no point in making use of that
> functionality.

At the end it's the decision of the GPIO maintainer.