On 1/8/2020 12:27 PM, Olivier Matz wrote: > On Wed, Jan 08, 2020 at 12:12:11PM +0000, Ferruh Yigit wrote: >> On 1/8/2020 9:42 AM, Olivier Matz wrote: >>> On Wed, Jan 08, 2020 at 09:09:29AM +0000, Ferruh Yigit wrote: >>>> On 1/8/2020 8:56 AM, David Marchand wrote: >>>>> Hello Laurent, >>>>> >>>>> Bonne année. >>>>> >>>>> Cc: maintainers. >>>>> >>>>> On Tue, Jan 7, 2020 at 3:57 PM Laurent Hardy <laurent.ha...@6wind.com> >>>>> wrote: >>>>>> >>>>>> In current led control API we have no way to know if a device is able >>>>>> to handle on/off requests coming from the application. >>>>>> Knowing if the device is led control capable could be useful to avoid >>>>>> exchanges between application and kernel. >>>>>> Using the on/off requests to flag if the device is led control capable >>>>>> (based on the ENOSUP returned error) is not convenient as such request >>>>>> can change the led state on device. >>>>>> >>>>>> This patch adds a new function rte_eth_led_ctrl_capable() that will look >>>>>> for led_off/on dev ops availability on the related pmd, to know if the >>>>>> device is able to handle such led control requests (on/off). >>>>> >>>>> This patch breaks the ABI, which is BAD :-). >>>> >>>> Why it is an ABI break, dev_ops should be between library and drivers, so >>>> it >>>> should be out of the ABI concern, isn't it. >>>> >>>>> This new api only needs to look at the existing ops, so you can remove >>>>> the (unused in your patch) dev_led_ctrl_capable ops. >>>>> >>>>> OTOH, would it make sense to expose this capability in dev_flags? >>>>> >>>> >>>> 'rte_eth_led_on()' & 'rte_eth_led_off()' APIs returns '-ENOTSUP' when the >>>> not >>>> supported, can that help application to understand? >>> >>> In our case, it is not possible to use rte_eth_led_on/off() to check if >>> the feature is supported: on success, it would change the value of the >>> led, and it seems it is not recoverable on some drivers. >> >> What does it mean it is not recoverable, like can you turn on the led but >> can't >> turn off it back? Can't this be fixed in the PMD? > > In the case there is only one LED, which is by default used to display > the link status, it can never display the status again without resetting > the device.
Is there a specific PMD are we talking about? > > Maybe an alternative solution would be to add a function, in addition to > on() and off(): > > led_ctrl_status_link() to display the status link on the led. > > >> >>> >>> Today it is not possible to know if the feature is available without >>> side effect. >>> >>
