Re: [PATCH 03/11] gpio: davinci: Modify to platform driver

[Sekhar Nori]

On 6/11/2013 6:25 PM, Philip, Avinash wrote:

> On Tue, Jun 11, 2013 at 17:26:06, Nori, Sekhar wrote:

>> On 5/22/2013 12:40 PM, Philip Avinash wrote:

>>> @@ -179,13 +204,10 @@ static int __init davinci_gpio_setup(void)
>>>  		gpiochip_add(&ctlrs[i].chip);
>>>  	}
>>>  
>>> -	soc_info->gpio_ctlrs = ctlrs;
>>
>>> -	soc_info->gpio_ctlrs_num = DIV_ROUND_UP(ngpio, 32);
>>
>> You drop setting gpio_ctlrs_num here and don't introduce it anywhere
>> else in the patchset so in effect you render the inline gpio get/set API
>> useless. Looks like this initialization should be moved to platform code?
> 
> With [PATCH 08/11] ARM: davinci: start using gpiolib support gpio get/set API
> Has no more dependency on soc_info->gpio_ctlrs_num.

With this series, you have removed support for inline gpio get/set API.
I see that the inline functions are still available for use on
tnetv107x. I wonder why it is important to keep these for tnetv107x when
not necessary for other DaVinci devices?

When you are removing this feature, please note it prominently in your
cover letter and patch description. Also, please provide some data on
how the latency now compares to that of inline access earlier. This is
important especially for the read. For the writes, gpio clock will
mostly determine how soon the value changes on the pin.

I am okay with removing the inline access feature. It helps simplify
code and most arm machines don't use them. I would just like to see some
data for justification as this can be seen as feature regression. Also,
if we are removing it, its better to also remove it completely and get
the LOC savings instead of just stopping its usage.

Thanks,
Sekhar
--
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/