diff mbox series

[RFC,net-next,4/8] dsa: Create port LEDs based on DT binding

Message ID 20231128232135.358638-5-andrew@lunn.ch (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series DSA LED infrastructure, mv88e6xxx and QCA8K | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/codegen success Generated files up to date
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1133 this patch: 1133
netdev/cc_maintainers warning 3 maintainers not CCed: kuba@kernel.org pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1148 this patch: 1148
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1160 this patch: 1160
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 200 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Andrew Lunn Nov. 28, 2023, 11:21 p.m. UTC 
Allow LEDs to be described in each ports DT subnode. Parse these when
setting up the ports, currently supporting brightness and link.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |   3 +
 net/dsa/dsa.c     | 138 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 insertions(+)

Comments

Christian Marangi Nov. 29, 2023, 1:46 a.m. UTC | #1 
On Wed, Nov 29, 2023 at 12:21:31AM +0100, Andrew Lunn wrote:
> Allow LEDs to be described in each ports DT subnode. Parse these when
> setting up the ports, currently supporting brightness and link.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/dsa.h |   3 +
>  net/dsa/dsa.c     | 138 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 141 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index ae73765cd71c..2e05e4fd0b76 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -325,6 +325,9 @@ struct dsa_port {
>  		 */
>  		struct list_head	user_vlans;
>  	};
> +
> +	/* List of LEDs associated to this port */
> +	struct list_head leds;
>  };
>  
>  /* TODO: ideally DSA ports would have a single dp->link_dp member,
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index ac7be864e80d..b13748f9b519 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -34,6 +34,15 @@
>  static DEFINE_MUTEX(dsa2_mutex);
>  LIST_HEAD(dsa_tree_list);
>  
> +struct dsa_led {
> +	struct list_head led_list;
> +	struct dsa_port *dp;
> +	struct led_classdev led_cdev;
> +	u8 index;
> +};
> +
> +#define to_dsa_led(d) container_of(d, struct dsa_led, led_cdev)
> +
>  static struct workqueue_struct *dsa_owq;
>  
>  /* Track the bridges with forwarding offload enabled */
> @@ -461,6 +470,116 @@ static void dsa_tree_teardown_cpu_ports(struct dsa_switch_tree *dst)
>  			dp->cpu_dp = NULL;
>  }
>  
> +static int dsa_led_brightness_set(struct led_classdev *led_cdev,
> +				  enum led_brightness value)
> +{
> +	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
> +	struct dsa_port *dp = dsa_led->dp;
> +	struct dsa_switch *ds = dp->ds;
> +
> +	return ds->ops->led_brightness_set(ds, dp->index, dsa_led->index,
> +					   value);
> +}
> +
> +static int dsa_led_blink_set(struct led_classdev *led_cdev,
> +			     unsigned long *delay_on, unsigned long *delay_off)
> +{
> +	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
> +	struct dsa_port *dp = dsa_led->dp;
> +	struct dsa_switch *ds = dp->ds;
> +
> +	return ds->ops->led_blink_set(ds, dp->index, dsa_led->index,
> +				      delay_on, delay_off);
> +}
> +
> +static int dsa_port_led_setup(struct dsa_port *dp,
> +			      struct device_node *led)
> +{
> +	struct led_init_data init_data = {};
> +	struct dsa_switch *ds = dp->ds;
> +	struct led_classdev *cdev;
> +	struct dsa_led *dsa_led;
> +	u32 index;
> +	int err;
> +
> +	dsa_led = devm_kzalloc(ds->dev, sizeof(*dsa_led), GFP_KERNEL);
> +	if (!dsa_led)
> +		return -ENOMEM;
> +
> +	dsa_led->dp = dp;
> +	cdev = &dsa_led->led_cdev;
> +
> +	err = of_property_read_u32(led, "reg", &index);
> +	if (err)
> +		return err;
> +	if (index > 255)
> +		return -EINVAL;
> +
> +	dsa_led->index = index;
> +
> +	if (ds->ops->led_brightness_set)
> +		cdev->brightness_set_blocking = dsa_led_brightness_set;
> +	if (ds->ops->led_blink_set)
> +		cdev->blink_set = dsa_led_blink_set;
> +
> +	cdev->max_brightness = 1;
> +
> +	init_data.fwnode = of_fwnode_handle(led);

Please add

init_data.devname_mandatory = true;

cled will derive the name based on color action and won't include the
devname resulting in LEDs having the same name.

> +	if (dp->user) {
> +		init_data.devicename = dev_name(&dp->user->dev);
> +		err = devm_led_classdev_register_ext(&dp->user->dev, cdev,
> +						     &init_data);
> +	} else {
> +		init_data.devicename = kasprintf(GFP_KERNEL, "%s:%d",
> +						 dev_name(ds->dev), dp->index);
> +		err = devm_led_classdev_register_ext(ds->dev, cdev, &init_data);
> +	}
> +	if (err)
> +		return err;
> +
> +	INIT_LIST_HEAD(&dsa_led->led_list);
> +	list_add(&dsa_led->led_list, &dp->leds);
> +
> +	if (!dp->user)
> +		kfree(init_data.devicename);
> +
> +	return 0;
> +}
> +
> +static int dsa_port_leds_setup(struct dsa_port *dp)
> +{
> +	struct device_node *leds, *led;
> +	int err;
> +
> +	if (!dp->dn)
> +		return 0;
> +
> +	leds = of_get_child_by_name(dp->dn, "leds");
> +	if (!leds)
> +		return 0;
> +
> +	for_each_available_child_of_node(leds, led) {
> +		err = dsa_port_led_setup(dp, led);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void dsa_port_leds_teardown(struct dsa_port *dp)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +	struct device *dev = ds->dev;
> +	struct led_classdev *cdev;
> +	struct dsa_led *dsa_led;
> +
> +	list_for_each_entry(dsa_led, &dp->leds, led_list) {
> +		cdev = &dsa_led->led_cdev;
> +		devm_led_classdev_unregister(dev, cdev);
> +	}
> +}
> +
>  static int dsa_port_setup(struct dsa_port *dp)
>  {
>  	bool dsa_port_link_registered = false;
> @@ -494,6 +613,11 @@ static int dsa_port_setup(struct dsa_port *dp)
>  		err = dsa_port_enable(dp, NULL);
>  		if (err)
>  			break;
> +
> +		err = dsa_port_leds_setup(dp);
> +		if (err)
> +			break;
> +
>  		dsa_port_enabled = true;
>  
>  		break;
> @@ -512,12 +636,22 @@ static int dsa_port_setup(struct dsa_port *dp)
>  		err = dsa_port_enable(dp, NULL);
>  		if (err)
>  			break;
> +
> +		err = dsa_port_leds_setup(dp);
> +		if (err)
> +			break;
> +
>  		dsa_port_enabled = true;
>  
>  		break;
>  	case DSA_PORT_TYPE_USER:
>  		of_get_mac_address(dp->dn, dp->mac);
>  		err = dsa_user_create(dp);
> +		if (err)
> +			break;
> +
> +		err = dsa_port_leds_setup(dp);
> +
>  		break;
>  	}
>  
> @@ -544,11 +678,13 @@ static void dsa_port_teardown(struct dsa_port *dp)
>  	case DSA_PORT_TYPE_UNUSED:
>  		break;
>  	case DSA_PORT_TYPE_CPU:
> +		dsa_port_leds_teardown(dp);
>  		dsa_port_disable(dp);
>  		if (dp->dn)
>  			dsa_shared_port_link_unregister_of(dp);
>  		break;
>  	case DSA_PORT_TYPE_DSA:
> +		dsa_port_leds_teardown(dp);
>  		dsa_port_disable(dp);
>  		if (dp->dn)
>  			dsa_shared_port_link_unregister_of(dp);
> @@ -556,6 +692,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
>  	case DSA_PORT_TYPE_USER:
>  		if (dp->user) {
>  			dsa_user_destroy(dp->user);
> +			dsa_port_leds_teardown(dp);
>  			dp->user = NULL;
>  		}
>  		break;
> @@ -1108,6 +1245,7 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
>  	INIT_LIST_HEAD(&dp->mdbs);
>  	INIT_LIST_HEAD(&dp->vlans); /* also initializes &dp->user_vlans */
>  	INIT_LIST_HEAD(&dp->list);
> +	INIT_LIST_HEAD(&dp->leds);
>  	list_add_tail(&dp->list, &dst->ports);
>  
>  	return dp;
> -- 
> 2.42.0
>
Simon Horman Nov. 29, 2023, 7:40 p.m. UTC | #2 
On Wed, Nov 29, 2023 at 12:21:31AM +0100, Andrew Lunn wrote:

...

> +static int dsa_port_leds_setup(struct dsa_port *dp)
> +{
> +	struct device_node *leds, *led;
> +	int err;
> +
> +	if (!dp->dn)
> +		return 0;
> +
> +	leds = of_get_child_by_name(dp->dn, "leds");
> +	if (!leds)
> +		return 0;
> +
> +	for_each_available_child_of_node(leds, led) {
> +		err = dsa_port_led_setup(dp, led);
> +		if (err)
> +			return err;

Hi Andrew,

I realise this is an RFC, but Coccinelle tells me that a call to
of_node_put() is needed here.

> +	}
> +
> +	return 0;
> +}

...
Andrew Lunn Nov. 29, 2023, 8:07 p.m. UTC | #3 
On Wed, Nov 29, 2023 at 07:40:28PM +0000, Simon Horman wrote:
> On Wed, Nov 29, 2023 at 12:21:31AM +0100, Andrew Lunn wrote:
> 
> ...
> 
> > +static int dsa_port_leds_setup(struct dsa_port *dp)
> > +{
> > +	struct device_node *leds, *led;
> > +	int err;
> > +
> > +	if (!dp->dn)
> > +		return 0;
> > +
> > +	leds = of_get_child_by_name(dp->dn, "leds");
> > +	if (!leds)
> > +		return 0;
> > +
> > +	for_each_available_child_of_node(leds, led) {
> > +		err = dsa_port_led_setup(dp, led);
> > +		if (err)
> > +			return err;
> 
> Hi Andrew,
> 
> I realise this is an RFC, but Coccinelle tells me that a call to
> of_node_put() is needed here.

Thanks. If you had not pointed it out, i would probably get it wrong
in the next version as well.

   Andrew
diff mbox series

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ae73765cd71c..2e05e4fd0b76 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -325,6 +325,9 @@  struct dsa_port {
 		 */
 		struct list_head	user_vlans;
 	};
+
+	/* List of LEDs associated to this port */
+	struct list_head leds;
 };
 
 /* TODO: ideally DSA ports would have a single dp->link_dp member,
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index ac7be864e80d..b13748f9b519 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -34,6 +34,15 @@ 
 static DEFINE_MUTEX(dsa2_mutex);
 LIST_HEAD(dsa_tree_list);
 
+struct dsa_led {
+	struct list_head led_list;
+	struct dsa_port *dp;
+	struct led_classdev led_cdev;
+	u8 index;
+};
+
+#define to_dsa_led(d) container_of(d, struct dsa_led, led_cdev)
+
 static struct workqueue_struct *dsa_owq;
 
 /* Track the bridges with forwarding offload enabled */
@@ -461,6 +470,116 @@  static void dsa_tree_teardown_cpu_ports(struct dsa_switch_tree *dst)
 			dp->cpu_dp = NULL;
 }
 
+static int dsa_led_brightness_set(struct led_classdev *led_cdev,
+				  enum led_brightness value)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_brightness_set(ds, dp->index, dsa_led->index,
+					   value);
+}
+
+static int dsa_led_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_blink_set(ds, dp->index, dsa_led->index,
+				      delay_on, delay_off);
+}
+
+static int dsa_port_led_setup(struct dsa_port *dp,
+			      struct device_node *led)
+{
+	struct led_init_data init_data = {};
+	struct dsa_switch *ds = dp->ds;
+	struct led_classdev *cdev;
+	struct dsa_led *dsa_led;
+	u32 index;
+	int err;
+
+	dsa_led = devm_kzalloc(ds->dev, sizeof(*dsa_led), GFP_KERNEL);
+	if (!dsa_led)
+		return -ENOMEM;
+
+	dsa_led->dp = dp;
+	cdev = &dsa_led->led_cdev;
+
+	err = of_property_read_u32(led, "reg", &index);
+	if (err)
+		return err;
+	if (index > 255)
+		return -EINVAL;
+
+	dsa_led->index = index;
+
+	if (ds->ops->led_brightness_set)
+		cdev->brightness_set_blocking = dsa_led_brightness_set;
+	if (ds->ops->led_blink_set)
+		cdev->blink_set = dsa_led_blink_set;
+
+	cdev->max_brightness = 1;
+
+	init_data.fwnode = of_fwnode_handle(led);
+	if (dp->user) {
+		init_data.devicename = dev_name(&dp->user->dev);
+		err = devm_led_classdev_register_ext(&dp->user->dev, cdev,
+						     &init_data);
+	} else {
+		init_data.devicename = kasprintf(GFP_KERNEL, "%s:%d",
+						 dev_name(ds->dev), dp->index);
+		err = devm_led_classdev_register_ext(ds->dev, cdev, &init_data);
+	}
+	if (err)
+		return err;
+
+	INIT_LIST_HEAD(&dsa_led->led_list);
+	list_add(&dsa_led->led_list, &dp->leds);
+
+	if (!dp->user)
+		kfree(init_data.devicename);
+
+	return 0;
+}
+
+static int dsa_port_leds_setup(struct dsa_port *dp)
+{
+	struct device_node *leds, *led;
+	int err;
+
+	if (!dp->dn)
+		return 0;
+
+	leds = of_get_child_by_name(dp->dn, "leds");
+	if (!leds)
+		return 0;
+
+	for_each_available_child_of_node(leds, led) {
+		err = dsa_port_led_setup(dp, led);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void dsa_port_leds_teardown(struct dsa_port *dp)
+{
+	struct dsa_switch *ds = dp->ds;
+	struct device *dev = ds->dev;
+	struct led_classdev *cdev;
+	struct dsa_led *dsa_led;
+
+	list_for_each_entry(dsa_led, &dp->leds, led_list) {
+		cdev = &dsa_led->led_cdev;
+		devm_led_classdev_unregister(dev, cdev);
+	}
+}
+
 static int dsa_port_setup(struct dsa_port *dp)
 {
 	bool dsa_port_link_registered = false;
@@ -494,6 +613,11 @@  static int dsa_port_setup(struct dsa_port *dp)
 		err = dsa_port_enable(dp, NULL);
 		if (err)
 			break;
+
+		err = dsa_port_leds_setup(dp);
+		if (err)
+			break;
+
 		dsa_port_enabled = true;
 
 		break;
@@ -512,12 +636,22 @@  static int dsa_port_setup(struct dsa_port *dp)
 		err = dsa_port_enable(dp, NULL);
 		if (err)
 			break;
+
+		err = dsa_port_leds_setup(dp);
+		if (err)
+			break;
+
 		dsa_port_enabled = true;
 
 		break;
 	case DSA_PORT_TYPE_USER:
 		of_get_mac_address(dp->dn, dp->mac);
 		err = dsa_user_create(dp);
+		if (err)
+			break;
+
+		err = dsa_port_leds_setup(dp);
+
 		break;
 	}
 
@@ -544,11 +678,13 @@  static void dsa_port_teardown(struct dsa_port *dp)
 	case DSA_PORT_TYPE_UNUSED:
 		break;
 	case DSA_PORT_TYPE_CPU:
+		dsa_port_leds_teardown(dp);
 		dsa_port_disable(dp);
 		if (dp->dn)
 			dsa_shared_port_link_unregister_of(dp);
 		break;
 	case DSA_PORT_TYPE_DSA:
+		dsa_port_leds_teardown(dp);
 		dsa_port_disable(dp);
 		if (dp->dn)
 			dsa_shared_port_link_unregister_of(dp);
@@ -556,6 +692,7 @@  static void dsa_port_teardown(struct dsa_port *dp)
 	case DSA_PORT_TYPE_USER:
 		if (dp->user) {
 			dsa_user_destroy(dp->user);
+			dsa_port_leds_teardown(dp);
 			dp->user = NULL;
 		}
 		break;
@@ -1108,6 +1245,7 @@  static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
 	INIT_LIST_HEAD(&dp->mdbs);
 	INIT_LIST_HEAD(&dp->vlans); /* also initializes &dp->user_vlans */
 	INIT_LIST_HEAD(&dp->list);
+	INIT_LIST_HEAD(&dp->leds);
 	list_add_tail(&dp->list, &dst->ports);
 
 	return dp;