Re: [PATCH v6 02/16] leds: triggers: let struct led_trigger::activate() return an error code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 29, 2018 at 10:30:37PM +0200, Uwe Kleine-König wrote:
> On Fri, Jun 29, 2018 at 10:13:34PM +0200, Jacek Anaszewski wrote:
> > Uwe,
> > 
> > Thanks for resending the patch set.
> > 
> > I tested it with x86_64_defconfig and got following warnings:
> > 
> > [...]
> >   CC      net/rfkill/core.o
> >   CC      net/netfilter/nf_conntrack_seqadj.o
> >   CC      net/netfilter/nf_conntrack_netlink.o
> > net/rfkill/core.c: In function ‘rfkill_led_trigger_register’:
> > net/rfkill/core.c:171:31: warning: assignment from incompatible pointer type
> >   rfkill->led_trigger.activate = rfkill_led_trigger_activate;
> >                                ^
> >   CC      net/ipv6/sysctl_net_ipv6.o
> >   CC      net/ipv4/tcp_recovery.o
> > [...]
> >   CC      net/mac80211/led.o
> >   CC      drivers/hid/hid-topseed.o
> >   CC      drivers/gpu/drm/i915/i915_gem_execbuffer.o
> >   CC      net/ipv4/netlink.o
> >   AR      drivers/hid/built-in.a
> >   CC      net/mac80211/pm.o
> > net/mac80211/led.c: In function ‘ieee80211_led_init’:
> > net/mac80211/led.c:148:25: warning: assignment from incompatible pointer
> > type
> >   local->rx_led.activate = ieee80211_rx_led_activate;
> >                          ^
> > net/mac80211/led.c:156:25: warning: assignment from incompatible pointer
> > type
> >   local->tx_led.activate = ieee80211_tx_led_activate;
> >                          ^
> > net/mac80211/led.c:164:28: warning: assignment from incompatible pointer
> > type
> >   local->assoc_led.activate = ieee80211_assoc_led_activate;
> >                             ^
> > net/mac80211/led.c:172:28: warning: assignment from incompatible pointer
> > type
> >   local->radio_led.activate = ieee80211_radio_led_activate;
> >                             ^
> > net/mac80211/led.c:181:27: warning: assignment from incompatible pointer
> > type
> >    local->tpt_led.activate = ieee80211_tpt_led_activate;
> > 
> > 
> > It seems that will need net maintainer acks anyway :-)
> 
> *sigh*, this wouldn't be a stopper if the conversion wouldn't have been
> done in a single step as was requested in (IIRC) v2 :-|

I found another one, the fixup is:

diff --git a/net/bluetooth/leds.c b/net/bluetooth/leds.c
index cb670b5594eb..6d59a5023231 100644
--- a/net/bluetooth/leds.c
+++ b/net/bluetooth/leds.c
@@ -43,7 +43,7 @@ void hci_leds_update_powered(struct hci_dev *hdev, bool enabled)
 	led_trigger_event(bt_power_led_trigger, enabled ? LED_FULL : LED_OFF);
 }
 
-static void power_activate(struct led_classdev *led_cdev)
+static int power_activate(struct led_classdev *led_cdev)
 {
 	struct hci_basic_led_trigger *htrig;
 	bool powered;
@@ -52,10 +52,12 @@ static void power_activate(struct led_classdev *led_cdev)
 	powered = test_bit(HCI_UP, &htrig->hdev->flags);
 
 	led_trigger_event(led_cdev->trigger, powered ? LED_FULL : LED_OFF);
+
+	return 0;
 }
 
 static struct led_trigger *led_allocate_basic(struct hci_dev *hdev,
-			void (*activate)(struct led_classdev *led_cdev),
+			int (*activate)(struct led_classdev *led_cdev),
 			const char *name)
 {
 	struct hci_basic_led_trigger *htrig;
diff --git a/net/mac80211/led.c b/net/mac80211/led.c
index ba0b507ea691..469a531e8d24 100644
--- a/net/mac80211/led.c
+++ b/net/mac80211/led.c
@@ -52,13 +52,15 @@ void ieee80211_free_led_names(struct ieee80211_local *local)
 	kfree(local->radio_led.name);
 }
 
-static void ieee80211_tx_led_activate(struct led_classdev *led_cdev)
+static int ieee80211_tx_led_activate(struct led_classdev *led_cdev)
 {
 	struct ieee80211_local *local = container_of(led_cdev->trigger,
 						     struct ieee80211_local,
 						     tx_led);
 
 	atomic_inc(&local->tx_led_active);
+
+	return 0
 }
 
 static void ieee80211_tx_led_deactivate(struct led_classdev *led_cdev)
@@ -70,13 +72,15 @@ static void ieee80211_tx_led_deactivate(struct led_classdev *led_cdev)
 	atomic_dec(&local->tx_led_active);
 }
 
-static void ieee80211_rx_led_activate(struct led_classdev *led_cdev)
+static int ieee80211_rx_led_activate(struct led_classdev *led_cdev)
 {
 	struct ieee80211_local *local = container_of(led_cdev->trigger,
 						     struct ieee80211_local,
 						     rx_led);
 
 	atomic_inc(&local->rx_led_active);
+
+	return 0;
 }
 
 static void ieee80211_rx_led_deactivate(struct led_classdev *led_cdev)
@@ -88,13 +92,15 @@ static void ieee80211_rx_led_deactivate(struct led_classdev *led_cdev)
 	atomic_dec(&local->rx_led_active);
 }
 
-static void ieee80211_assoc_led_activate(struct led_classdev *led_cdev)
+static int ieee80211_assoc_led_activate(struct led_classdev *led_cdev)
 {
 	struct ieee80211_local *local = container_of(led_cdev->trigger,
 						     struct ieee80211_local,
 						     assoc_led);
 
 	atomic_inc(&local->assoc_led_active);
+
+	return 0;
 }
 
 static void ieee80211_assoc_led_deactivate(struct led_classdev *led_cdev)
@@ -106,13 +112,15 @@ static void ieee80211_assoc_led_deactivate(struct led_classdev *led_cdev)
 	atomic_dec(&local->assoc_led_active);
 }
 
-static void ieee80211_radio_led_activate(struct led_classdev *led_cdev)
+static int ieee80211_radio_led_activate(struct led_classdev *led_cdev)
 {
 	struct ieee80211_local *local = container_of(led_cdev->trigger,
 						     struct ieee80211_local,
 						     radio_led);
 
 	atomic_inc(&local->radio_led_active);
+
+	return 0;
 }
 
 static void ieee80211_radio_led_deactivate(struct led_classdev *led_cdev)
@@ -124,13 +132,15 @@ static void ieee80211_radio_led_deactivate(struct led_classdev *led_cdev)
 	atomic_dec(&local->radio_led_active);
 }
 
-static void ieee80211_tpt_led_activate(struct led_classdev *led_cdev)
+static int ieee80211_tpt_led_activate(struct led_classdev *led_cdev)
 {
 	struct ieee80211_local *local = container_of(led_cdev->trigger,
 						     struct ieee80211_local,
 						     tpt_led);
 
 	atomic_inc(&local->tpt_led_active);
+
+	return 0;
 }
 
 static void ieee80211_tpt_led_deactivate(struct led_classdev *led_cdev)
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index a7a4e6ff9be2..1355f5ca8d22 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -141,13 +141,15 @@ static void rfkill_led_trigger_event(struct rfkill *rfkill)
 		led_trigger_event(trigger, LED_FULL);
 }
 
-static void rfkill_led_trigger_activate(struct led_classdev *led)
+static int rfkill_led_trigger_activate(struct led_classdev *led)
 {
 	struct rfkill *rfkill;
 
 	rfkill = container_of(led->trigger, struct rfkill, led_trigger);
 
 	rfkill_led_trigger_event(rfkill);
+
+	return 0;
 }
 
 const char *rfkill_get_led_trigger_name(struct rfkill *rfkill)


I fixed my branch at

	https://git.pengutronix.de/git/ukl/linux led-trigger

and wonder about how to proceed? Do you fixup v6 patch accordingly,
should I send a v7? If the latter, only this single patch or the whole
series? Or do you want to pull?

If there are some more that slipped through my build tests and grep
patterns I tend to go back to introducing a new member for the
conversion to be able to get the patches in and be able to revert a
single patch when another problem appears without having to throw out
the whole series.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux