Re: [PATCH RFC lora-next 1/4] net: lora: sx125x sx1301: correct style warnings

2019-01-04 Thread Ben Whitten 

Hi Andreas,

On 29/12/2018 09:05, Andreas Färber wrote:

Hi Ben,

Am 19.12.18 um 16:56 schrieb Ben Whitten:

Checkpatch highlights some style issues which need to be addressed.

Signed-off-by: Ben Whitten 
---
  drivers/net/lora/sx125x.c | 20 +--
  drivers/net/lora/sx1301.c | 52 ++-
  drivers/net/lora/sx1301.h |  7 +++---
  3 files changed, 45 insertions(+), 34 deletions(-)


Thanks for splitting this off from the functional changes. This will
need some more explanations and discussion. In particular the comment
changes look wrong to me. Also please don't butcher code just because
checkpatch.pl may warn about line length at this early stage.


Yeh they seemed odd but checkpatch doesn't like a /* on a line by its 
self it seems.




A good catch in there was the unsigned casts, which are no longer
necessary since the regmap conversion, so we should just squash that
into the earlier commits.

Note that I used to run checkpatch.pl as git commit hook:
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
But since some git update that started to break git rebase -i in case of
false positives (with rebase stopping and on trying to continue commits
unintentionally getting squashed), so I deactivated it again and don't
manually check each commit I stage anymore, in favor of slowly
completing implementations to a working point.


Good to know, I'll avoid running it for the time and drop this from the v2

Thanks!
Ben Whitten

Re: [PATCH RFC lora-next 1/4] net: lora: sx125x sx1301: correct style warnings

2018-12-28 Thread Andreas Färber 
Hi Ben,

Am 19.12.18 um 16:56 schrieb Ben Whitten:
> Checkpatch highlights some style issues which need to be addressed.
> 
> Signed-off-by: Ben Whitten 
> ---
>  drivers/net/lora/sx125x.c | 20 +--
>  drivers/net/lora/sx1301.c | 52 ++-
>  drivers/net/lora/sx1301.h |  7 +++---
>  3 files changed, 45 insertions(+), 34 deletions(-)

Thanks for splitting this off from the functional changes. This will
need some more explanations and discussion. In particular the comment
changes look wrong to me. Also please don't butcher code just because
checkpatch.pl may warn about line length at this early stage.

A good catch in there was the unsigned casts, which are no longer
necessary since the regmap conversion, so we should just squash that
into the earlier commits.

Note that I used to run checkpatch.pl as git commit hook:
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
But since some git update that started to break git rebase -i in case of
false positives (with rebase stopping and on trying to continue commits
unintentionally getting squashed), so I deactivated it again and don't
manually check each commit I stage anymore, in favor of slowly
completing implementations to a working point.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[PATCH RFC lora-next 1/4] net: lora: sx125x sx1301: correct style warnings

2018-12-19 Thread Ben Whitten 
Checkpatch highlights some style issues which need to be addressed.

Signed-off-by: Ben Whitten 
---
 drivers/net/lora/sx125x.c | 20 +--
 drivers/net/lora/sx1301.c | 52 ++-
 drivers/net/lora/sx1301.h |  7 +++---
 3 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
index b7ca782b9386..1a941f663c52 100644
--- a/drivers/net/lora/sx125x.c
+++ b/drivers/net/lora/sx125x.c
@@ -49,7 +49,7 @@ struct sx125x_priv {
 
struct device   *dev;
struct regmap   *regmap;
-   struct regmap_field 
*regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
+   struct regmap_field *regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
 };
 
 #define to_clkout(_hw) container_of(_hw, struct sx125x_priv, clkout_hw)
@@ -67,13 +67,13 @@ static struct regmap_config __maybe_unused 
sx125x_regmap_config = {
 };
 
 static int sx125x_field_write(struct sx125x_priv *priv,
-   enum sx125x_fields field_id, u8 val)
+ enum sx125x_fields field_id, u8 val)
 {
return regmap_field_write(priv->regmap_fields[field_id], val);
 }
 
 static int sx125x_field_read(struct sx125x_priv *priv,
-   enum sx125x_fields field_id, unsigned int *val)
+enum sx125x_fields field_id, unsigned int *val)
 {
return regmap_field_read(priv->regmap_fields[field_id], val);
 }
@@ -149,8 +149,12 @@ static int sx125x_register_clock_provider(struct 
sx125x_priv *priv)
init.num_parents = 1;
priv->clkout_hw.init = 
 
-   of_property_read_string_index(dev->of_node, "clock-output-names", 0,
-   );
+   ret = of_property_read_string_index(dev->of_node, "clock-output-names",
+   0, );
+   if (ret) {
+   dev_err(dev, "unable to find output name\n");
+   return ret;
+   }
 
priv->clkout = devm_clk_register(dev, >clkout_hw);
if (IS_ERR(priv->clkout)) {
@@ -158,7 +162,7 @@ static int sx125x_register_clock_provider(struct 
sx125x_priv *priv)
return PTR_ERR(priv->clkout);
}
ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
-   >clkout_hw);
+>clkout_hw);
return ret;
 }
 
@@ -180,8 +184,8 @@ static int __maybe_unused sx125x_regmap_probe(struct device 
*dev, struct regmap
const struct reg_field *reg_fields = sx125x_regmap_fields;
 
priv->regmap_fields[i] = devm_regmap_field_alloc(dev,
-   priv->regmap,
-   reg_fields[i]);
+priv->regmap,
+reg_fields[i]);
if (IS_ERR(priv->regmap_fields[i])) {
ret = PTR_ERR(priv->regmap_fields[i]);
dev_err(dev, "Cannot allocate regmap field: %d\n", ret);
diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 23cbddc364e5..e75df93b96d8 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Semtech SX1301 LoRa concentrator
+/* Semtech SX1301 LoRa concentrator
  *
  * Copyright (c) 2018 Andreas Färber
  * Copyright (c) 2018 Ben Whitten
@@ -55,12 +54,13 @@ static struct regmap_config sx1301_regmap_config = {
 };
 
 static int sx1301_field_write(struct sx1301_priv *priv,
-   enum sx1301_fields field_id, u8 val)
+ enum sx1301_fields field_id, u8 val)
 {
return regmap_field_write(priv->regmap_fields[field_id], val);
 }
 
-static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr, unsigned int 
*val)
+static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr,
+  unsigned int *val)
 {
int ret;
 
@@ -79,7 +79,8 @@ static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 
addr, unsigned int *
return 0;
 }
 
-static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, unsigned int 
*val)
+static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr,
+  unsigned int *val)
 {
int ret;
 
@@ -98,7 +99,8 @@ static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 
addr, unsigned int *
return 0;
 }
 
-static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const 
struct firmware *fw)
+static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu,
+   const struct firmware *fw)
 {
u8 *buf;
enum sx1301_fields rst, select_mux;
@@ -165,7 +167,8 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, 
int mcu, const struct
}
 
if (memcmp(fw->data, buf, fw->size)) {
-