drivers: misc: ti-st: fix review comments

Based on comments from Jiri Slaby, drop the register storage specifier, remove the unused code, cleanup the const to non-const type casting. Also make the line discipline ops structure static, since its a singleton, unmodified structure which need not be in heap. Reported-by: Jiri Slaby <jirislaby@gmail.com> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers: misc: ti-st: fix review comments
Based on comments from Jiri Slaby, drop the register storage specifier, remove the unused code, cleanup the const to non-const type casting. Also make the line discipline ops structure static, since its a singleton, unmodified structure which need not be in heap. Reported-by: Jiri Slaby <jirislaby@gmail.com> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Pavan Savoy · Greg Kroah-Hartman
1 parent da6830ac39
Showing 3 changed files with 25 additions and 65 deletions Side-by-side Diff
drivers/misc/ti-st/st_core.c
drivers/misc/ti-st/st_kim.c
include/linux/ti_wilink_st.h
@@ -31,15 +31,6 @@
 #include <net/bluetooth/hci.h>
 #include <linux/ti_wilink_st.h>
  
-/* strings to be used for rfkill entries and by
- * ST Core to be used for sysfs debug entry
- */
-#define PROTO_ENTRY(type, name)	name
-const unsigned char *protocol_strngs[] = {
-	PROTO_ENTRY(ST_BT, "Bluetooth"),
-	PROTO_ENTRY(ST_FM, "FM"),
-	PROTO_ENTRY(ST_GPS, "GPS"),
-};
 /* function pointer pointing to either,
  * st_kim_recv during registration to receive fw download responses
  * st_int_recv after registration to receive proto stack responses
@@ -144,7 +135,7 @@
 static inline int st_check_data_len(struct st_data_s *st_gdata,
 	int protoid, int len)
 {
-	register int room = skb_tailroom(st_gdata->rx_skb);
+	int room = skb_tailroom(st_gdata->rx_skb);
  
 	pr_debug("len %d room %d", len, room);
  
@@ -187,7 +178,7 @@
 static inline void st_wakeup_ack(struct st_data_s *st_gdata,
 	unsigned char cmd)
 {
-	register struct sk_buff *waiting_skb;
+	struct sk_buff *waiting_skb;
 	unsigned long flags = 0;
  
 	spin_lock_irqsave(&st_gdata->lock, flags);
  
@@ -216,13 +207,13 @@
 void st_int_recv(void *disc_data,
 	const unsigned char *data, long count)
 {
-	register char *ptr;
+	char *ptr;
 	struct hci_event_hdr *eh;
 	struct hci_acl_hdr *ah;
 	struct hci_sco_hdr *sh;
 	struct fm_event_hdr *fm;
 	struct gps_event_hdr *gps;
-	register int len = 0, type = 0, dlen = 0;
+	int len = 0, type = 0, dlen = 0;
 	static enum proto_type protoid = ST_MAX;
 	struct st_data_s *st_gdata = (struct st_data_s *)disc_data;
  
  
  
  
@@ -918,34 +909,27 @@
 	return;
 }
  
+static struct tty_ldisc_ops st_ldisc_ops = {
+	.magic = TTY_LDISC_MAGIC,
+	.name = "n_st",
+	.open = st_tty_open,
+	.close = st_tty_close,
+	.receive_buf = st_tty_receive,
+	.write_wakeup = st_tty_wakeup,
+	.flush_buffer = st_tty_flush_buffer,
+	.owner = THIS_MODULE
+};
+
 /********************************************************************/
 int st_core_init(struct st_data_s **core_data)
 {
 	struct st_data_s *st_gdata;
 	long err;
-	static struct tty_ldisc_ops *st_ldisc_ops;
  
-	/* populate and register to TTY line discipline */
-	st_ldisc_ops = kzalloc(sizeof(*st_ldisc_ops), GFP_KERNEL);
-	if (!st_ldisc_ops) {
-		pr_err("no mem to allocate");
-		return -ENOMEM;
-	}
-
-	st_ldisc_ops->magic = TTY_LDISC_MAGIC;
-	st_ldisc_ops->name = "n_st";	/*"n_hci"; */
-	st_ldisc_ops->open = st_tty_open;
-	st_ldisc_ops->close = st_tty_close;
-	st_ldisc_ops->receive_buf = st_tty_receive;
-	st_ldisc_ops->write_wakeup = st_tty_wakeup;
-	st_ldisc_ops->flush_buffer = st_tty_flush_buffer;
-	st_ldisc_ops->owner = THIS_MODULE;
-
-	err = tty_register_ldisc(N_TI_WL, st_ldisc_ops);
+	err = tty_register_ldisc(N_TI_WL, &st_ldisc_ops);
 	if (err) {
 		pr_err("error registering %d line discipline %ld",
 			   N_TI_WL, err);
-		kfree(st_ldisc_ops);
 		return err;
 	}
 	pr_debug("registered n_shared line discipline");
@@ -956,7 +940,6 @@
 		err = tty_unregister_ldisc(N_TI_WL);
 		if (err)
 			pr_err("unable to un-register ldisc %ld", err);
-		kfree(st_ldisc_ops);
 		err = -ENOMEM;
 		return err;
 	}
@@ -970,22 +953,6 @@
 	/* Locking used in st_int_enqueue() to avoid multiple execution */
 	spin_lock_init(&st_gdata->lock);
  
-	/* ldisc_ops ref to be only used in __exit of module */
-	st_gdata->ldisc_ops = st_ldisc_ops;
-
-#if 0
-	err = st_kim_init();
-	if (err) {
-		pr_err("error during kim initialization(%ld)", err);
-		kfree(st_gdata);
-		err = tty_unregister_ldisc(N_TI_WL);
-		if (err)
-			pr_err("unable to un-register ldisc");
-		kfree(st_ldisc_ops);
-		return -1;
-	}
-#endif
-
 	err = st_ll_init(st_gdata);
 	if (err) {
 		pr_err("error during st_ll initialization(%ld)", err);
@@ -993,7 +960,6 @@
 		err = tty_unregister_ldisc(N_TI_WL);
 		if (err)
 			pr_err("unable to un-register ldisc");
-		kfree(st_ldisc_ops);
 		return -1;
 	}
 	*core_data = st_gdata;
@@ -1007,11 +973,7 @@
 	err = st_ll_deinit(st_gdata);
 	if (err)
 		pr_err("error during deinit of ST LL %ld", err);
-#if 0
-	err = st_kim_deinit();
-	if (err)
-		pr_err("error during deinit of ST KIM %ld", err);
-#endif
+
 	if (st_gdata != NULL) {
 		/* Free ST Tx Qs and skbs */
 		skb_queue_purge(&st_gdata->txq);
@@ -1022,7 +984,6 @@
 		err = tty_unregister_ldisc(N_TI_WL);
 		if (err)
 			pr_err("unable to un-register ldisc %ld", err);
-		kfree(st_gdata->ldisc_ops);
 		/* free the global data pointer */
 		kfree(st_gdata);
 	}
@@ -75,7 +75,7 @@
 };
  
 #define MAX_ST_DEVICES	3	/* Imagine 1 on each UART for now */
-struct platform_device *st_kim_devices[MAX_ST_DEVICES];
+static struct platform_device *st_kim_devices[MAX_ST_DEVICES];
  
 /**********************************************************************/
 /* internal functions */
  
  
  
@@ -157,17 +157,18 @@
 void kim_int_recv(struct kim_data_s *kim_gdata,
 	const unsigned char *data, long count)
 {
-	register char *ptr;
+	const unsigned char *ptr;
 	struct hci_event_hdr *eh;
-	register int len = 0, type = 0;
+	int len = 0, type = 0;
  
 	pr_debug("%s", __func__);
 	/* Decode received bytes here */
-	ptr = (char *)data;
+	ptr = data;
 	if (unlikely(ptr == NULL)) {
 		pr_err(" received null from TTY ");
 		return;
 	}
+
 	while (count) {
 		if (kim_gdata->rx_count) {
 			len = min_t(unsigned int, kim_gdata->rx_count, count);
@@ -231,7 +232,7 @@
 static long read_local_version(struct kim_data_s *kim_gdata, char *bts_scr_name)
 {
 	unsigned short version = 0, chip = 0, min_ver = 0, maj_ver = 0;
-	char read_ver_cmd[] = { 0x01, 0x01, 0x10, 0x00 };
+	const char read_ver_cmd[] = { 0x01, 0x01, 0x10, 0x00 };
  
 	pr_debug("%s", __func__);
  
@@ -278,8 +279,8 @@
 {
 	long err = 0;
 	long len = 0;
-	register unsigned char *ptr = NULL;
-	register unsigned char *action_ptr = NULL;
+	unsigned char *ptr = NULL;
+	unsigned char *action_ptr = NULL;
 	unsigned char bts_scr_name[30] = { 0 };	/* 30 char long bts scr name? */
  
 	err = read_local_version(kim_gdata, bts_scr_name);
@@ -101,7 +101,6 @@
  *	can occur , where as during other times other events CH8, CH9
  *	can occur.
  * @tty: tty provided by the TTY core for line disciplines.
- * @ldisc_ops: the procedures that this line discipline registers with TTY.
  * @tx_skb: If for some reason the tty's write returns lesser bytes written
  *	then to maintain the rest of data to be written on next instance.
  *	This needs to be protected, hence the lock inside wakeup func.
@@ -132,7 +131,6 @@
 struct st_data_s {
 	unsigned long st_state;
 	struct tty_struct *tty;
-	struct tty_ldisc_ops *ldisc_ops;
 	struct sk_buff *tx_skb;
 #define ST_TX_SENDING	1
 #define ST_TX_WAKEUP	2
...	...	@@ -31,15 +31,6 @@
31	31	#include <net/bluetooth/hci.h>
32	32	#include <linux/ti_wilink_st.h>
33	33
34		-/* strings to be used for rfkill entries and by
35		- * ST Core to be used for sysfs debug entry
36		- */
37		-#define PROTO_ENTRY(type, name) name
38		-const unsigned char *protocol_strngs[] = {
39		- PROTO_ENTRY(ST_BT, "Bluetooth"),
40		- PROTO_ENTRY(ST_FM, "FM"),
41		- PROTO_ENTRY(ST_GPS, "GPS"),
42		-};
43	34	/* function pointer pointing to either,
44	35	* st_kim_recv during registration to receive fw download responses
45	36	* st_int_recv after registration to receive proto stack responses
...	...	@@ -144,7 +135,7 @@
144	135	static inline int st_check_data_len(struct st_data_s *st_gdata,
145	136	int protoid, int len)
146	137	{
147		- register int room = skb_tailroom(st_gdata->rx_skb);
	138	+ int room = skb_tailroom(st_gdata->rx_skb);
148	139
149	140	pr_debug("len %d room %d", len, room);
150	141
...	...	@@ -187,7 +178,7 @@
187	178	static inline void st_wakeup_ack(struct st_data_s *st_gdata,
188	179	unsigned char cmd)
189	180	{
190		- register struct sk_buff *waiting_skb;
	181	+ struct sk_buff *waiting_skb;
191	182	unsigned long flags = 0;
192	183
193	184	spin_lock_irqsave(&st_gdata->lock, flags);
194	185
...	...	@@ -216,13 +207,13 @@
216	207	void st_int_recv(void *disc_data,
217	208	const unsigned char *data, long count)
218	209	{
219		- register char *ptr;
	210	+ char *ptr;
220	211	struct hci_event_hdr *eh;
221	212	struct hci_acl_hdr *ah;
222	213	struct hci_sco_hdr *sh;
223	214	struct fm_event_hdr *fm;
224	215	struct gps_event_hdr *gps;
225		- register int len = 0, type = 0, dlen = 0;
	216	+ int len = 0, type = 0, dlen = 0;
226	217	static enum proto_type protoid = ST_MAX;
227	218	struct st_data_s st_gdata = (struct st_data_s )disc_data;
228	219
229	220
230	221
231	222
...	...	@@ -918,34 +909,27 @@
918	909	return;
919	910	}
920	911
	912	+static struct tty_ldisc_ops st_ldisc_ops = {
	913	+ .magic = TTY_LDISC_MAGIC,
	914	+ .name = "n_st",
	915	+ .open = st_tty_open,
	916	+ .close = st_tty_close,
	917	+ .receive_buf = st_tty_receive,
	918	+ .write_wakeup = st_tty_wakeup,
	919	+ .flush_buffer = st_tty_flush_buffer,
	920	+ .owner = THIS_MODULE
	921	+};
	922	+
921	923	/********************************************************************/
922	924	int st_core_init(struct st_data_s **core_data)
923	925	{
924	926	struct st_data_s *st_gdata;
925	927	long err;
926		- static struct tty_ldisc_ops *st_ldisc_ops;
927	928
928		- /* populate and register to TTY line discipline */
929		- st_ldisc_ops = kzalloc(sizeof(*st_ldisc_ops), GFP_KERNEL);
930		- if (!st_ldisc_ops) {
931		- pr_err("no mem to allocate");
932		- return -ENOMEM;
933		- }
934		-
935		- st_ldisc_ops->magic = TTY_LDISC_MAGIC;
936		- st_ldisc_ops->name = "n_st"; /"n_hci"; /
937		- st_ldisc_ops->open = st_tty_open;
938		- st_ldisc_ops->close = st_tty_close;
939		- st_ldisc_ops->receive_buf = st_tty_receive;
940		- st_ldisc_ops->write_wakeup = st_tty_wakeup;
941		- st_ldisc_ops->flush_buffer = st_tty_flush_buffer;
942		- st_ldisc_ops->owner = THIS_MODULE;
943		-
944		- err = tty_register_ldisc(N_TI_WL, st_ldisc_ops);
	929	+ err = tty_register_ldisc(N_TI_WL, &st_ldisc_ops);
945	930	if (err) {
946	931	pr_err("error registering %d line discipline %ld",
947	932	N_TI_WL, err);
948		- kfree(st_ldisc_ops);
949	933	return err;
950	934	}
951	935	pr_debug("registered n_shared line discipline");
...	...	@@ -956,7 +940,6 @@
956	940	err = tty_unregister_ldisc(N_TI_WL);
957	941	if (err)
958	942	pr_err("unable to un-register ldisc %ld", err);
959		- kfree(st_ldisc_ops);
960	943	err = -ENOMEM;
961	944	return err;
962	945	}
...	...	@@ -970,22 +953,6 @@
970	953	/* Locking used in st_int_enqueue() to avoid multiple execution */
971	954	spin_lock_init(&st_gdata->lock);
972	955
973		- /* ldisc_ops ref to be only used in __exit of module */
974		- st_gdata->ldisc_ops = st_ldisc_ops;
975		-
976		-#if 0
977		- err = st_kim_init();
978		- if (err) {
979		- pr_err("error during kim initialization(%ld)", err);
980		- kfree(st_gdata);
981		- err = tty_unregister_ldisc(N_TI_WL);
982		- if (err)
983		- pr_err("unable to un-register ldisc");
984		- kfree(st_ldisc_ops);
985		- return -1;
986		- }
987		-#endif
988		-
989	956	err = st_ll_init(st_gdata);
990	957	if (err) {
991	958	pr_err("error during st_ll initialization(%ld)", err);
...	...	@@ -993,7 +960,6 @@
993	960	err = tty_unregister_ldisc(N_TI_WL);
994	961	if (err)
995	962	pr_err("unable to un-register ldisc");
996		- kfree(st_ldisc_ops);
997	963	return -1;
998	964	}
999	965	*core_data = st_gdata;
...	...	@@ -1007,11 +973,7 @@
1007	973	err = st_ll_deinit(st_gdata);
1008	974	if (err)
1009	975	pr_err("error during deinit of ST LL %ld", err);
1010		-#if 0
1011		- err = st_kim_deinit();
1012		- if (err)
1013		- pr_err("error during deinit of ST KIM %ld", err);
1014		-#endif
	976	+
1015	977	if (st_gdata != NULL) {
1016	978	/* Free ST Tx Qs and skbs */
1017	979	skb_queue_purge(&st_gdata->txq);
...	...	@@ -1022,7 +984,6 @@
1022	984	err = tty_unregister_ldisc(N_TI_WL);
1023	985	if (err)
1024	986	pr_err("unable to un-register ldisc %ld", err);
1025		- kfree(st_gdata->ldisc_ops);
1026	987	/* free the global data pointer */
1027	988	kfree(st_gdata);
1028	989	}
...	...	@@ -75,7 +75,7 @@
75	75	};
76	76
77	77	#define MAX_ST_DEVICES 3 /* Imagine 1 on each UART for now */
78		-struct platform_device *st_kim_devices[MAX_ST_DEVICES];
	78	+static struct platform_device *st_kim_devices[MAX_ST_DEVICES];
79	79
80	80	/**********************************************************************/
81	81	/* internal functions */
82	82
83	83
84	84
...	...	@@ -157,17 +157,18 @@
157	157	void kim_int_recv(struct kim_data_s *kim_gdata,
158	158	const unsigned char *data, long count)
159	159	{
160		- register char *ptr;
	160	+ const unsigned char *ptr;
161	161	struct hci_event_hdr *eh;
162		- register int len = 0, type = 0;
	162	+ int len = 0, type = 0;
163	163
164	164	pr_debug("%s", __func__);
165	165	/* Decode received bytes here */
166		- ptr = (char *)data;
	166	+ ptr = data;
167	167	if (unlikely(ptr == NULL)) {
168	168	pr_err(" received null from TTY ");
169	169	return;
170	170	}
	171	+
171	172	while (count) {
172	173	if (kim_gdata->rx_count) {
173	174	len = min_t(unsigned int, kim_gdata->rx_count, count);
...	...	@@ -231,7 +232,7 @@
231	232	static long read_local_version(struct kim_data_s kim_gdata, char bts_scr_name)
232	233	{
233	234	unsigned short version = 0, chip = 0, min_ver = 0, maj_ver = 0;
234		- char read_ver_cmd[] = { 0x01, 0x01, 0x10, 0x00 };
	235	+ const char read_ver_cmd[] = { 0x01, 0x01, 0x10, 0x00 };
235	236
236	237	pr_debug("%s", __func__);
237	238
...	...	@@ -278,8 +279,8 @@
278	279	{
279	280	long err = 0;
280	281	long len = 0;
281		- register unsigned char *ptr = NULL;
282		- register unsigned char *action_ptr = NULL;
	282	+ unsigned char *ptr = NULL;
	283	+ unsigned char *action_ptr = NULL;
283	284	unsigned char bts_scr_name[30] = { 0 }; /* 30 char long bts scr name? */
284	285
285	286	err = read_local_version(kim_gdata, bts_scr_name);
...	...	@@ -101,7 +101,6 @@
101	101	* can occur , where as during other times other events CH8, CH9
102	102	* can occur.
103	103	* @tty: tty provided by the TTY core for line disciplines.
104		- * @ldisc_ops: the procedures that this line discipline registers with TTY.
105	104	* @tx_skb: If for some reason the tty's write returns lesser bytes written
106	105	* then to maintain the rest of data to be written on next instance.
107	106	* This needs to be protected, hence the lock inside wakeup func.
...	...	@@ -132,7 +131,6 @@
132	131	struct st_data_s {
133	132	unsigned long st_state;
134	133	struct tty_struct *tty;
135		- struct tty_ldisc_ops *ldisc_ops;
136	134	struct sk_buff *tx_skb;
137	135	#define ST_TX_SENDING 1
138	136	#define ST_TX_WAKEUP 2