Re: [PATCH v3 3/3] gpio: add support for Cypress CYUSBS234 USB-GPIO adapter

From: RR
Date: Tue Oct 28 2014 - 22:21:13 EST


On Mon, Oct 27, 2014 at 9:00 PM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Thu, Oct 9, 2014 at 1:46 AM, RR <rajaram.officemails@xxxxxxxxx> wrote:
>> On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:
>>> On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani <muth@xxxxxxxxxxx> wrote:
>
>>>>> > +static int cy_gpio_direction_output(struct gpio_chip *chip,
>>>>> > + unsigned offset, int value) {
>>>>> > + return 0;
>>>>> > +}
>>>>>
>>>>> If that chip is capable of both output and input, shouldn't these functions be
>>>>> implemented? I think this has already been pointed out in a previous version
>>>>> but you did not reply.
>>>>
>>>> Thanks for your inputs.
>>>>
>>>> Only the GPIOs which are configured to be output GPIO can be set.
>>>
>>> In that case cy_gpio_set() should return an error for GPIOs which are
>>> not configured as outputs. Is that guaranteed by the current
>>> implementation?
>>>
>>>> The set operation would fail trying to set the input or unconfigured GPIOs.
>>>> In this version of driver, this support is not added, it can be introduced in future versions.
>>>> I will add a TODO note in the code.
>>>
>>> Argh, no TODO please. Actual code that will turn this code into a
>>> solid driver that can be merged.
>>
>> Does a driver targeted for a custom device has to implement every
>> functionality in the 1st version ?
>
> When you post a driver to the GPIO maintainers it is *NOT* tageted
> at a consumer device, it is targeted at the kernel community and
> upstream maintainers.

Totally agree. What I was conveying the patch has not modified
any "core" kernel function and is specific to a device thus will not
affect system.

>
> Of course you can deliver add-on patches out-of-tree to your
> customers, it's generally a bad idea for the long term and maintenance
> of your driver, but it's your pick.

AFAIR In the recent past xHCI or gadget core or musb or dw3
patches were added in increments. May be my analogy is incorrect and
I am ignorant of some philosophy here.

Sincerely I somehow was not convinced basic functionality is missing
as referred in the review comment.We have tested the driver for most of
the functionality of our DVK and is working perfectly.

Moreover currently we do not expect an user to set gpio direction as
it involves vendor specific usb control commands.

Having said that we have taken the feedback and working to close this.

>
> 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/