Felipe Balbi <balbi@xxxxxx> writes:
> Hi,
>
> On Wed, Sep 24, 2014 at 10:14:53PM +0200, Robert Jarzmik wrote:
> if you reply like this again I'll start ignoring your patches. Settle
> down, I'm trying to help you get your patches merged.
>
> With that said:
>
> patch 1 should *ONLY* convert gpio_* to gpiod_*. That turns the patch
> into a clear cleanup where people don't need to spend too much time
> reviewing details.
>
>> >> - convert the mach info, and store the udc_command in the pxa_udc
>> >> control structure
>
> this is patch 2.
So be it, I'll send it right after this mail.
>
>> >> - loose the udc_is_connected() in mach info
>> >> This was not really used, as mioa701 machine doesn't need it,
>> >> balloon3 doesn't really use it, and most importantly the current
>> >> driver never uses it.
>
> and this is patch 3.
This is no patch, there is not a single line in the patch for that. This is just
a statement, I can remove it if you want.
>
>> >> The drawback with the gpio_desc conversion is that the "inverted" gpio
>> >> attribute is lost, as no gpiod_*() function exists to set the
>> >
>> > it's not lost, it's handled for you by the gpio library. Look at how
>> > GPIO_ACTIVE_LOW is used.
>> It is so, the above assertion "no gpiod_*() function exists" is
>> false. Therefore, which is the right function in the gpio library accessible to
>> a driver ?
>
> look at the implementation of gpiod_set_value:
>
> 1271 void gpiod_set_value(struct gpio_desc *desc, int value)
> 1272 {
> 1273 if (!desc)
> 1274 return;
> 1275 /* Should be using gpio_set_value_cansleep() */
> 1276 WARN_ON(desc->chip->can_sleep);
> 1277 if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
> 1278 value = !value;
> 1279 _gpiod_set_raw_value(desc, value);
> 1280 }
> 1281 EXPORT_SYMBOL_GPL(gpiod_set_value);
>
> do you see how it handles our "inverted" flag internally. It seems like
> what you're missing is a helper to set FLAG_ACTIVE_LOW when !OF &&
> !ACPI. If that's really not part of the framework yet, propose a patch
> as that would help many others convert to gpio descs and, later, DT.
You still don't get my point I'm afraid, sic... It's not about "using" the flag,
it's about "setting" the flag from the driver. Or said differently the assertion
is "there is not gpiod_*() function which toggles the FLAG_ACTIVE_LOW bit id
gpio_desc->flags".
I'll wait for Linus on that topic.
--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
