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

[Santosh Shilimkar <santosh.shilimkar@xxxxxx>]

On Thursday 24 July 2014 02:52 PM, Suman Anna wrote:
> Hi,
> 
> On 07/24/2014 01:12 PM, Jassi Brar wrote:
>> On 24 July 2014 22:52, Santosh Shilimkar <santosh.shilimkar@xxxxxx> wrote:
>>> On Thursday 24 July 2014 01:19 PM, Jassi Brar wrote:
>>>> On 23 July 2014 20:40, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>>>>> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
>>>>> <grygorii.strashko@xxxxxx> wrote:
>>>>>
>>>>>> From: Murali Karicheri <m-karicheri2@xxxxxx>
>>>>>>
>>>>>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>>>>>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>>>>>> each DSP core. This is one of the component used by the IPC mechanism used
>>>>>> on Keystone SOCs.
>>>>>>
>>>>>> Keystone 2 DSP GPIO controller has specific features:
>>>>>> - each GPIO can be configured only as output pin;
>>>>>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>>>>>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>>>>>   pending.
>>>>>>
>>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@xxxxxx>
>>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@xxxxxx>
>>>>>
>>>>> Pardon me. How is this GENERAL PURPOSE Input/Output?
>>>>>
>>>>> It seems very very much SPECIAL PURPOSE to me, it's like
>>>>> you're just shoehorning some IPC mechanism into the GPIO
>>>>> subsystem, and this may be because the datasheet calls it
>>>>> GPIO when it's not.
>>>>>
>>>>> What other stuff than DSP is connected to these lines, and is it
>>>>> really even external lines? Aren't these just polysilicon rails
>>>>> pretty much hammered to be used by the DSP and nothing else.
>>>>>
>>>>> What is the difference between this and a mailbox IRQ line
>>>>> and the kind of stuff handled by drivers/mailbox?
>>>>>
>>>>> I'd like Suman and Jassi to have a look at this to see if it's
>>>>> actually a mailbox before we proceed.
>>>>>
>>>> The controller seems like most others, only incapable of reading
>>>> signals (output only).
>>>>  The userspace driving those signals to communicate with a DSP isn't
>>>> enough to call it a mailbox usecase, because on a different board the
>>>> userspace may drive those signals to control LEDs :)
>>>>
>>> Exactly !!
>>> And that was my point. Thanks for echo.
>>>
>> Yeah but if the AP and DSP are within the same package (i.e, the
>> 'pins' can't be used for any other purpose on any board), one might
>> sell it as a mailbox. However, since the mailbox protocol driver would
>> be in userspace, I think it is justified to expose that as GPIO
>> otherwise we'll have to add another interface for userspace to control
>> the DSP.
> 
> If all these pins or a group of pins are collectively being used to
> denote a message to the DSP, then I can see the argument for this being
> a mailbox platform driver. It then probably would need to be
> supplemented by some other protocol driver to expose the necessary
> functionality to userspace. Otherwise, I agree with Jassi in general.
>
Just 1 line is used as I mentioned to you off-list already.
 
> That said, having two different drivers for different GPIO IP instances
> within the same SoC doesn't make sense. There should be a single driver
> for all the GPIO IPs in keystone, output/input only are properties of
> the instance and the driver should handle that IMHO.
> 
I wish the hardware integration was the way you said. The one GPIO
controller supports only 32 IO's and thats used for IO's. The IP
block was carried over from Legacy SOCs. The $subject IP block 
has sligtly different programming model, address space, clock domains
etc so there is no question of handling that in 1 driver where the
two hardware IPs are different. Hope its clear why they are in
two separate drivers.

Regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html