Re: [PATCH] Staging: gdm724x: LTE: Fix trailing open parentheses.

[Dan Carpenter]

This patch is fine.

Acked-by: Dan Carpenter 

But I have a some comments for later.

On Wed, Feb 21, 2018 at 02:20:17AM -0800, Quytelda Kahja wrote:
> @@ -509,8 +511,9 @@ static struct net_device_stats *gdm_lte_stats(struct 
> net_device *dev)
>  
>  static int gdm_lte_event_send(struct net_device *dev, char *buf, int len)
>  {
> - struct nic *nic = netdev_priv(dev);
> + struct phy_dev *phy_dev = ((struct nic *)netdev_priv(dev))->phy_dev;
  ^
This is a bit unsightly.  Better to make it two assignments:

struct nic *nic = netdev_priv(dev);
struct phy_dev *phy_dev = nic->phy_dev;

>   struct hci_packet *hci = (struct hci_packet *)buf;
> + int length;
>   int idx;
>   int ret;
>  
> @@ -518,11 +521,9 @@ static int gdm_lte_event_send(struct net_device *dev, 
> char *buf, int len)
>   if (ret != 1)
>   return -EINVAL;
>  
> - return netlink_send(lte_event.sock, idx, 0, buf,
> - gdm_dev16_to_cpu(
> - nic->phy_dev->get_endian(
> - nic->phy_dev->priv_dev), hci->len)
> - + HCI_HEADER_SIZE);
> + length = gdm_dev16_to_cpu(phy_dev->get_endian(phy_dev->priv_dev),
> +   hci->len) + HCI_HEADER_SIZE;
> + return netlink_send(lte_event.sock, idx, 0, buf, length);

It would be nicer to store:

struct gdm_endian *ed = phy_dev->get_endian(phy_dev->priv_dev);

at the start of the function as well.  Then this looks like:

length = gdm_dev16_to_cpu(ed, hci->len) + HCI_HEADER_SIZE;
netlink_send(lte_event.sock, idx, 0, buf, length);

The endian information doesn't need to be a struct any more since
we remove ed->host_ed in 77e8a50149a2 ("staging: gdm724x: Remove test
for host endian").  We should change gdm_dev16_to_cpu() to just take
dev_ed.

>  }
>  
>  static void gdm_lte_event_rcv(struct net_device *dev, u16 type,
> @@ -731,15 +732,13 @@ static void gdm_lte_pdn_table(struct net_device *dev, 
> char *buf, int len)
>   struct hci_pdn_table_ind *pdn_table = (struct hci_pdn_table_ind *)buf;
>  
>   if (pdn_table->activate) {
> + struct gdm_endian *ed;
> +
>   nic->pdn_table.activate = pdn_table->activate;
> - nic->pdn_table.dft_eps_id = gdm_dev32_to_cpu(
> - nic->phy_dev->get_endian(
> - nic->phy_dev->priv_dev),
> - pdn_table->dft_eps_id);
> - nic->pdn_table.nic_type = gdm_dev32_to_cpu(
> - nic->phy_dev->get_endian(
> - nic->phy_dev->priv_dev),
> - pdn_table->nic_type);
> +
> + ed = nic->phy_dev->get_endian(nic->phy_dev->priv_dev);
> + nic->pdn_table.dft_eps_id = gdm_dev32_to_cpu(ed, 
> pdn_table->dft_eps_id);
> + nic->pdn_table.nic_type = gdm_dev32_to_cpu(ed, 
> pdn_table->nic_type);
>  
>   netdev_info(dev, "pdn activated, nic_type=0x%x\n",
>   nic->pdn_table.nic_type);

Use the same three initial variables here:

struct nic *nic = netdev_priv(dev);
struct phy_dev *phy_dev = nic->phy_dev;
struct gdm_endian *ed = phy_dev->get_endian(nic->phy_dev->priv_dev);

Then reverse the ->active test:

if (!pdn_table->activate) {
memset(>pdn_table, 0, sizeof(struct pdn_table));
netdev_info(dev, "pdn deactivated\n");
return;
}

Then pull everything in a tab:

nic->pdn_table.activate = pdn_table->activate;
nic->pdn_table.dft_eps_id = gdm_dev32_to_cpu(ed, pdn_table->dft_eps_id);
nic->pdn_table.nic_type = gdm_dev32_to_cpu(ed, pdn_table->nic_type);


regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] Staging: gdm724x: LTE: Fix trailing open parentheses.

[Quytelda Kahja]

Fix lines with a trailing open parenthesis, which is a coding style issue.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/gdm724x/gdm_lte.c | 44 +++
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_lte.c 
b/drivers/staging/gdm724x/gdm_lte.c
index a6608637035a..8d492d6d6a12 100644
--- a/drivers/staging/gdm724x/gdm_lte.c
+++ b/drivers/staging/gdm724x/gdm_lte.c
@@ -185,6 +185,7 @@ static __sum16 icmp6_checksum(struct ipv6hdr *ipv6, u16 
*ptr, int len)
unsigned short *w = ptr;
__wsum sum = 0;
int i;
+   u16 pa;
 
union {
struct {
@@ -204,9 +205,10 @@ static __sum16 icmp6_checksum(struct ipv6hdr *ipv6, u16 
*ptr, int len)
pseudo_header.ph.ph_nxt = ipv6->nexthdr;
 
w = (u16 *)_header;
-   for (i = 0; i < ARRAY_SIZE(pseudo_header.pa); i++)
-   sum = csum_add(sum, csum_unfold(
-   (__force __sum16)pseudo_header.pa[i]));
+   for (i = 0; i < ARRAY_SIZE(pseudo_header.pa); i++) {
+   pa = pseudo_header.pa[i];
+   sum = csum_add(sum, csum_unfold((__force __sum16)pa));
+   }
 
w = ptr;
while (len > 1) {
@@ -509,8 +511,9 @@ static struct net_device_stats *gdm_lte_stats(struct 
net_device *dev)
 
 static int gdm_lte_event_send(struct net_device *dev, char *buf, int len)
 {
-   struct nic *nic = netdev_priv(dev);
+   struct phy_dev *phy_dev = ((struct nic *)netdev_priv(dev))->phy_dev;
struct hci_packet *hci = (struct hci_packet *)buf;
+   int length;
int idx;
int ret;
 
@@ -518,11 +521,9 @@ static int gdm_lte_event_send(struct net_device *dev, char 
*buf, int len)
if (ret != 1)
return -EINVAL;
 
-   return netlink_send(lte_event.sock, idx, 0, buf,
-   gdm_dev16_to_cpu(
-   nic->phy_dev->get_endian(
-   nic->phy_dev->priv_dev), hci->len)
-   + HCI_HEADER_SIZE);
+   length = gdm_dev16_to_cpu(phy_dev->get_endian(phy_dev->priv_dev),
+ hci->len) + HCI_HEADER_SIZE;
+   return netlink_send(lte_event.sock, idx, 0, buf, length);
 }
 
 static void gdm_lte_event_rcv(struct net_device *dev, u16 type,
@@ -731,15 +732,13 @@ static void gdm_lte_pdn_table(struct net_device *dev, 
char *buf, int len)
struct hci_pdn_table_ind *pdn_table = (struct hci_pdn_table_ind *)buf;
 
if (pdn_table->activate) {
+   struct gdm_endian *ed;
+
nic->pdn_table.activate = pdn_table->activate;
-   nic->pdn_table.dft_eps_id = gdm_dev32_to_cpu(
-   nic->phy_dev->get_endian(
-   nic->phy_dev->priv_dev),
-   pdn_table->dft_eps_id);
-   nic->pdn_table.nic_type = gdm_dev32_to_cpu(
-   nic->phy_dev->get_endian(
-   nic->phy_dev->priv_dev),
-   pdn_table->nic_type);
+
+   ed = nic->phy_dev->get_endian(nic->phy_dev->priv_dev);
+   nic->pdn_table.dft_eps_id = gdm_dev32_to_cpu(ed, 
pdn_table->dft_eps_id);
+   nic->pdn_table.nic_type = gdm_dev32_to_cpu(ed, 
pdn_table->nic_type);
 
netdev_info(dev, "pdn activated, nic_type=0x%x\n",
nic->pdn_table.nic_type);
@@ -897,12 +896,11 @@ int register_lte_device(struct phy_dev *phy_dev,
nic->phy_dev = phy_dev;
nic->nic_id = index;
 
-   form_mac_address(
-   net->dev_addr,
-   nic->src_mac_addr,
-   nic->dest_mac_addr,
-   mac_address,
-   index);
+   form_mac_address(net->dev_addr,
+nic->src_mac_addr,
+nic->dest_mac_addr,
+mac_address,
+index);
 
SET_NETDEV_DEV(net, dev);
SET_NETDEV_DEVTYPE(net, _type);
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel