> If we should reuse blink_set to control hw blink I need to understand 2 > thing. > > The idea to implement the function hw_control_configure was to provide > the triggers some way to configure the various blink_mode. (and by using > the cmd enum provide different info based on the return value) > > The advised path from Marek is to make the changes in the trigger_data > and the LED driver will then use that to configure blink mode. > > I need to call an example to explain my concern: > qca8k switch. Can both run in hardware mode and software mode (by > controlling the reg to trigger manual blink) and also there is an extra > mode to blink permanently at 4hz. > > Now someone would declare the brightness_set to control the led > manually and blink_set (for the permanent 4hz blink feature) > So that's where my idea comes about introducting another function and > the fact that it wouldn't match that well with blink_set with some LED. > > I mean if we really want to use blink_set also for this(hw_control), we > can consider adding a bool to understand when hw_control is active or not. > So blink_set can be used for both feature. I don't see why we need the bool. The driver should know that speeds it supports. If asked to do something it cannot do in the current mode it should return either -EINVAL, or maybe -EOPNOTSUPP. Depending on how to the trigger works, we might want -EOPNOTSUPP when in a hardware offload mode, which gets returned to user space. If we are in a software blinking mode -EINVAL, so that the trigger does the blinking in software. Andrew
