> > > +#ifdef CONFIG_LEDS_HARDWARE_CONTROL > > > +enum blink_mode_cmd { > > > + BLINK_MODE_ENABLE, /* Enable the hardware blink mode */ > > > + BLINK_MODE_DISABLE, /* Disable the hardware blink mode */ > > > + BLINK_MODE_READ, /* Read the status of the hardware blink mode */ > > > + BLINK_MODE_SUPPORTED, /* Ask the driver if the hardware blink mode is supported */ > > > + BLINK_MODE_ZERO, /* Disable any hardware blink active */ > > > +}; > > > +#endif > > > > this is a strange proposal for the API. > > > > Anyway, led_classdev already has the blink_set() method, which is documented as > > /* > > * Activate hardware accelerated blink, delays are in milliseconds > > * and if both are zero then a sensible default should be chosen. > > * The call should adjust the timings in that case and if it can't > > * match the values specified exactly. > > * Deactivate blinking again when the brightness is set to LED_OFF > > * via the brightness_set() callback. > > */ > > int (*blink_set)(struct led_classdev *led_cdev, > > unsigned long *delay_on, > > unsigned long *delay_off); > > > > So we already have a method to set hardware blkinking, we don't need > > another one. > > > > Marek > > But that is about hardware blink, not a LED controlled by hardware based > on some rules/modes. > Doesn't really match the use for the hardware control. > Blink_set makes the LED blink contantly at the declared delay. > The blink_mode_cmd are used to request stuff to a LED in hardware mode. > > Doesn't seem correct to change/enhance the blink_set function with > something that would do something completely different. Humm. I can see merits for both. What i like about reusing blink_set() is that it is well understood. There is a defined sysfs API for it. ledtrig-oneshot.c also uses it, for a non-repeating blink. So i think that also fits the PHY LED use case. Andrew