My view on the subject and I think we need this resolved ASAP:

Keeping APIs stable is crucial, the question is, how do we get to stable APIs and how do we maintain progress while keeping those stable. 1.0 was released as the first publicly available version of Zephyr with APIs that have been introduced without any public scrutiny and with a limited set of usage scenarios, given the plans for the project to move to a less frequent release cycle (every 3 months) and the intention to maintain a long term support branch later on and in a less frequent manner I think our true target for a stable API would but such a release and not the initial 1.0.

Having said that and going forward we need to establish a strong process for defining and changing APIs to avoid such deadlocks and the introduction of compatibility layers that would increase complexity of code and increase code size as well in some cases.

Any disagreements here? If none, I propose to accept those changes proposed by Tomasz and set a target and a plan for API stability in the project TSC.

Regards,
Anas

Guys,

Thoughts or answers on that one:

Question: *_are we comfortable with preserving app compatibility, but
breaking driver compatibility?_* I am not so sure about this but would
like some feedback from the larger group.

Tomasz

Hi Andrew,

Another question: I had asked you to mark gpio_set_callback() with
__attribute__((deprecated)), but you said that broke the build. Can
you provide a little more detail here? is it because the sample
applications haven't been updated to the new API yet (I saw them later
in the patch series). Would it be possible to provide a new patch in
the series, after all the sample apps have been updated, to add the
__attribute__((deprecated)) tag there?

Ok I fixed that so the first patch still keeps the old behavior enabled
by default in Kconfig, so CI does not scream.
I did not wanted to merge all the adaptation patches on top with it, to
keep the history clear.

And last patch, as you advised, deprecate the call and disable by
default the old behavior in Kconfib

Tomasz

Hi Andrew,

Thank you for you thorough review, it sums up nicely the patchset.

Question: *_are we comfortable with preserving app compatibility, but
breaking driver compatibility?_* I am not so sure about this but would
like some feedback from the larger group.

This question is really critical.
If we have to support all API layers from 1.0: app, driver's interface,
low level kernel API - device for instance - etc.... things are going to
be much harder when introducing changes.

Another question: I had asked you to mark gpio_set_callback() with
__attribute__((deprecated)), but you said that broke the build. Can
you provide a little more detail here? is it because the sample
applications haven't been updated to the new API yet (I saw them later
in the patch series). Would it be possible to provide a new patch in
the series, after all the sample apps have been updated, to add the
__attribute__((deprecated)) tag there?

As far as I remember (CI does not have the logs, it drops them after
some time), it was failing due to gpio_api_compat.c which was
implementing the deprecated function, using the deprecated gpio_callback_t

That said, I could disable by default such compat layer, and set the
deprecated in a last patch yes. Should fix this issue.

Tomasz

Hi Tomasz,

This is in reference to https://gerrit.zephyrproject.org/r/#/c/914/

I wanted to discuss on-list some of my concerns, and this really leads into a larger topic on how we should handle changing APIs.

Summary (as I can understand it) what the patch does:
The old way of registering a GPIO callback was through the gpio_set_callback() API. It takes a function pointer of type gpio_callback_t. You can only set one callback handler in the entire driver. This is clearly not ideal.

The new way of registering a GPIO callback is through gpio_register_callback(), which will let you associate a handler function with a specific set of pins. It takes a struct gpio_callback as its argument. You can also un-register callbacks with gpio_unregister_callback(), and there is a helper function gpio_init_callback() for setting up the struct gpio_callback.

The incompatible change is the modification of gpio_set_callback_t in the API struct; it has fundamentally different semantics with the new driver API.

The implementation of gpio_set_callback() has changed. It is now located in a new C file. There is some magic going on: if new-style drivers want to support the old 1.0 API, they turn on CONFIG_GPIO_API_1_0_COMPATIBILITY and then declare GPIO_SETUP_COMPAT_DEV() somewhere in their driver file. This will create some storage space in a special read/write section for the old-style gpio_callback_t and argument.

The net effect of this is as follows:

1. Applications that have not been ported to use the new GPIO API, will still work as CONFIG_GPIO_API_1_0_COMPATIBILITY is turned on by default. It does require that any new GPIO drivers add GPIO_SETUP_COMPAT_DEV().

2. As far as I can tell, old-style GPIO drivers that are not in the Zephyr tree will be broken by this change as gpio_set_callback() will no longer call the relevant function in the API struct, and the definition of gpio_set_callback_t has changed anyway.

Question: are we comfortable with preserving app compatibility, but breaking driver compatibility? I am not so sure about this but would like some feedback from the larger group.

Another question: I had asked you to mark gpio_set_callback() with __attribute__((deprecated)), but you said that broke the build. Can you provide a little more detail here? is it because the sample applications haven't been updated to the new API yet (I saw them later in the patch series). Would it be possible to provide a new patch in the series, after all the sample apps have been updated, to add the __attribute__((deprecated)) tag there?

Andrew