Hi,
small nitpick at the end of the patch ...
On 12/02/2016 00:45, Michal wrote:
> From: Michal Cieslakiewicz <michal.cieslakiew...@wp.pl>
> Subject: [PATCH 2/4] switch: allow Ethernet port LEDs to show specific link
> speeds only
>
> This patch adds speed_mask special file to LEDs connected to switch ports
> via 'switch' trigger. It allows to choose which speeds to signal when link
> is up. If router has more than one LED per port, they may light up differently
> depending on how fast connection is. Default setting is 'all speeds' so
> backward compatibilty with system scripts (for example uci) is maintained.
>
> Signed-off-by: Michal Cieslakiewicz <michal.cieslakiew...@wp.pl>
> ---
> Link speed to speed_mask bit mapping:
> bit 0 (0x01) - unknown port speed, may indicate cabling problem
> bit 1 (0x02) - 10 Mbps
> bit 2 (0x04) - 100 Mbps
> bit 3 (0x08) - 1000 Mbps aka 1 Gbps
> bits 4-7 - reserved for future use, always 0, ignored for now
>
> .../generic/files/drivers/net/phy/swconfig_leds.c | 106
> ++++++++++++++++++++-
> 1 file changed, 102 insertions(+), 4 deletions(-)
>
> diff --git a/target/linux/generic/files/drivers/net/phy/swconfig_leds.c
> b/target/linux/generic/files/drivers/net/phy/swconfig_leds.c
> index 7d122d2..433a472 100644
> --- a/target/linux/generic/files/drivers/net/phy/swconfig_leds.c
> +++ b/target/linux/generic/files/drivers/net/phy/swconfig_leds.c
> @@ -20,6 +20,15 @@
> #define SWCONFIG_LED_TIMER_INTERVAL (HZ / 10)
> #define SWCONFIG_LED_NUM_PORTS 32
>
> +#define SWCONFIG_LED_PORT_SPEED_NA 0x01 /* unknown speed */
> +#define SWCONFIG_LED_PORT_SPEED_10 0x02 /* 10 Mbps */
> +#define SWCONFIG_LED_PORT_SPEED_100 0x04 /* 100 Mbps */
> +#define SWCONFIG_LED_PORT_SPEED_1000 0x08 /* 1000 Mbps */
> +#define SWCONFIG_LED_PORT_SPEED_ALL ( SWCONFIG_LED_PORT_SPEED_NA | \
> + SWCONFIG_LED_PORT_SPEED_10 | \
> + SWCONFIG_LED_PORT_SPEED_100 | \
> + SWCONFIG_LED_PORT_SPEED_1000 )
> +
> struct switch_led_trigger {
> struct led_trigger trig;
> struct switch_dev *swdev;
> @@ -28,6 +37,7 @@ struct switch_led_trigger {
> u32 port_mask;
> u32 port_link;
> unsigned long port_traffic[SWCONFIG_LED_NUM_PORTS];
> + u8 link_speed[SWCONFIG_LED_NUM_PORTS];
> };
>
> struct swconfig_trig_data {
> @@ -40,6 +50,7 @@ struct swconfig_trig_data {
> bool prev_link;
> unsigned long prev_traffic;
> enum led_brightness prev_brightness;
> + u8 speed_mask;
> };
>
> static void
> @@ -135,6 +146,46 @@ swconfig_trig_port_mask_show(struct device *dev, struct
> device_attribute *attr,
> static DEVICE_ATTR(port_mask, 0644, swconfig_trig_port_mask_show,
> swconfig_trig_port_mask_store);
>
> +/* speed_mask file handler - display value */
> +static ssize_t swconfig_trig_speed_mask_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct swconfig_trig_data *trig_data = led_cdev->trigger_data;
> +
> + read_lock(&trig_data->lock);
> + sprintf(buf, "%#x\n", trig_data->speed_mask);
> + read_unlock(&trig_data->lock);
> +
> + return strlen(buf) + 1;
> +}
> +
> +/* speed_mask file handler - store value */
> +static ssize_t swconfig_trig_speed_mask_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct swconfig_trig_data *trig_data = led_cdev->trigger_data;
> + u8 speed_mask;
> + int ret;
> +
> + ret = kstrtou8(buf, 0, &speed_mask);
> + if (ret)
> + return ret;
> +
> + write_lock(&trig_data->lock);
> + trig_data->speed_mask = speed_mask & SWCONFIG_LED_PORT_SPEED_ALL;
> + write_unlock(&trig_data->lock);
> +
> + return size;
> +}
> +
> +/* speed_mask special file */
> +static DEVICE_ATTR(speed_mask, 0644, swconfig_trig_speed_mask_show,
> + swconfig_trig_speed_mask_store);
> +
> static void
> swconfig_trig_activate(struct led_classdev *led_cdev)
> {
> @@ -154,14 +205,22 @@ swconfig_trig_activate(struct led_classdev *led_cdev)
> rwlock_init(&trig_data->lock);
> trig_data->led_cdev = led_cdev;
> trig_data->swdev = sw_trig->swdev;
> + trig_data->speed_mask = SWCONFIG_LED_PORT_SPEED_ALL;
> led_cdev->trigger_data = trig_data;
>
> err = device_create_file(led_cdev->dev, &dev_attr_port_mask);
> if (err)
> goto err_free;
>
> + err = device_create_file(led_cdev->dev, &dev_attr_speed_mask);
> + if (err)
> + goto err_dev_free;
> +
> return;
>
> +err_dev_free:
> + device_remove_file(led_cdev->dev, &dev_attr_port_mask);
> +
> err_free:
> led_cdev->trigger_data = NULL;
> kfree(trig_data);
> @@ -177,6 +236,7 @@ swconfig_trig_deactivate(struct led_classdev *led_cdev)
> trig_data = (void *) led_cdev->trigger_data;
> if (trig_data) {
> device_remove_file(led_cdev->dev, &dev_attr_port_mask);
> + device_remove_file(led_cdev->dev, &dev_attr_speed_mask);
> kfree(trig_data);
> }
> }
> @@ -188,6 +248,7 @@ swconfig_trig_led_event(struct switch_led_trigger
> *sw_trig,
> struct swconfig_trig_data *trig_data;
> u32 port_mask;
> bool link;
> + u8 speed_mask;
>
> trig_data = led_cdev->trigger_data;
> if (!trig_data)
> @@ -195,6 +256,7 @@ swconfig_trig_led_event(struct switch_led_trigger
> *sw_trig,
>
> read_lock(&trig_data->lock);
> port_mask = trig_data->port_mask;
> + speed_mask = trig_data->speed_mask;
> read_unlock(&trig_data->lock);
>
> link = !!(sw_trig->port_link & port_mask);
> @@ -203,17 +265,30 @@ swconfig_trig_led_event(struct switch_led_trigger
> *sw_trig,
> swconfig_trig_set_brightness(trig_data, LED_OFF);
> } else {
> unsigned long traffic;
> + bool speedok;
> int i;
>
> traffic = 0;
> + speedok = 0;
> for (i = 0; i < SWCONFIG_LED_NUM_PORTS; i++) {
> if (port_mask & (1 << i))
> - traffic += sw_trig->port_traffic[i];
> + if (sw_trig->link_speed[i] & speed_mask)
> + {
> + traffic += sw_trig->port_traffic[i];
> + speedok = 1;
> + }
> }
>
> - if (trig_data->prev_brightness != LED_FULL)
> - swconfig_trig_set_brightness(trig_data, LED_FULL);
> - else if (traffic != trig_data->prev_traffic)
> + if (speedok)
> + {
> + if (trig_data->prev_brightness != LED_FULL)
> + swconfig_trig_set_brightness(trig_data,
> + LED_FULL);
> + else if (traffic != trig_data->prev_traffic)
> + swconfig_trig_set_brightness(trig_data,
> + LED_OFF);
> + }
> + else if (trig_data->prev_brightness != LED_OFF)
> swconfig_trig_set_brightness(trig_data, LED_OFF);
>
> trig_data->prev_traffic = traffic;
> @@ -258,6 +333,8 @@ swconfig_led_work_func(struct work_struct *work)
> for (i = 0; i < SWCONFIG_LED_NUM_PORTS; i++) {
> u32 port_bit;
>
> + sw_trig->link_speed[i] = 0;
> +
> port_bit = BIT(i);
> if ((port_mask & port_bit) == 0)
> continue;
> @@ -269,7 +346,28 @@ swconfig_led_work_func(struct work_struct *work)
> swdev->ops->get_port_link(swdev, i, &port_link);
>
> if (port_link.link)
> + {
put the bracket on the same line as the if clause
> link |= port_bit;
> + switch(port_link.speed)
> + {
put the bracket on the same line as the switch
> + case SWITCH_PORT_SPEED_UNKNOWN:
case statements should be indented one tab less so that they are aligned
with the switch
John
> + sw_trig->link_speed[i] =
> + SWCONFIG_LED_PORT_SPEED_NA;
> + break;
> + case SWITCH_PORT_SPEED_10:
> + sw_trig->link_speed[i] =
> + SWCONFIG_LED_PORT_SPEED_10;
> + break;
> + case SWITCH_PORT_SPEED_100:
> + sw_trig->link_speed[i] =
> + SWCONFIG_LED_PORT_SPEED_100;
> + break;
> + case SWITCH_PORT_SPEED_1000:
> + sw_trig->link_speed[i] =
> + SWCONFIG_LED_PORT_SPEED_1000;
> + break;
> + }
> + }
> }
>
> if (swdev->ops->get_port_stats) {
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
