Re: [PATCH v1] gpio: keystone: add dsp gpio controller driver

[Linus Walleij]

On Thu, Jul 24, 2014 at 4:21 PM, Santosh Shilimkar
<santosh.shilimkar@xxxxxx> wrote:
> On Thursday 24 July 2014 10:12 AM, Linus Walleij wrote:
>> On Wed, Jul 23, 2014 at 5:25 PM, Santosh Shilimkar
>> <santosh.shilimkar@xxxxxx> wrote:
>>
>>> I will try to answer this. This IP is indeed a GPIO block
>>> but the IO's are used just OUTPUT lines from Linux
>>> HOST perspective. These IOs are connected to the DSPs
>>> as input/IRQ lines.
>>
>> So the DSP is another discrete IC, and could be something
>> different, so this is board-level information?
>>
>> I'm really worrying whether this is general purpose or not :-/
>>
> Am not sure I follow you. This IP is completely controlled
> by Linux OS to generate output signals. How does it matter
> whether its connected to a peripheral or a discrete IC.

What matters to me is whether it is general purpose or
not, and what the use case is.

The Kconfig symbol is called GPIO_KEYSTONE_DSP
not GPIO_KEYSTONE. That does not sound very general
purpose at all. Why is "DSP" at the end of that config
option if it is general purpose?

And we know the Keystone
already has another GPIO block, selected from the
Kconfig symbol GPIO_DAVINCI. Probably that is the
only real GPIO on this system.

And the use case doesn't seem to be exactly for
things like driving leds, reading keys, bit-banging SPI
or MMC card detect or other such common cases.
It seems to be to trigger IRQs on another processor and
nothing else. Not general purpose.

If writing bit such and such in some register has the
effect of pulling a bit high or low in some other IP
is not GPIO. It should be part of the driver for that
other IP block.

Further you wrote:

> The DSP-ARM host IPC mechanism used on
> Keystone is Linux user-space based and it does as
> one of the component.

And given that it's already hinted that this is actually
only there to aid some userspace driver I'm even *less*
interested in having it in the GPIO subsystem.

Shoehorning this into the GPIO subsystem just seems
like some convenient way to keep that DSP driver code
in userspace instead of writing a proper kernel driver
for the DSP.

Like someone wants to avoid things like this:
drivers/staging/tidspbridge
drivers/remoteproc/omap_remoteproc.c
drivers/remoteproc/da8xx_remoteproc.c

As a community maintainer, naturally doing such real
kernel drivers is way better than trying to sneak something
in under the radar, and now I'm worried that this is what
is actually attempted by this driver, so I'm concerned.

> Given that this IP only output functionality is used but
> that shouldn't matter. We have seen SOCs where GPIOs
> are just used as input to form a Matrix Keyboard.

Yes, that is common. And that is for example done with
GPIO_DAVINCI which has lines out to open space
and general purposes.

This is not GPIO, this is DSPIO IMO.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/