All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4.4 02/97] parisc: Fix out of array access in match_pci_device()
From: Greg Kroah-Hartman @ 2018-04-22 13:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Helge Deller
In-Reply-To: <20180422135304.577223025@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Helge Deller <deller@gmx.de>

commit 615b2665fd20c327b631ff1e79426775de748094 upstream.

As found by the ubsan checker, the value of the 'index' variable can be
out of range for the bc[] array:

UBSAN: Undefined behaviour in arch/parisc/kernel/drivers.c:655:21
index 6 is out of range for type 'char [6]'
Backtrace:
 [<104fa850>] __ubsan_handle_out_of_bounds+0x68/0x80
 [<1019d83c>] check_parent+0xc0/0x170
 [<1019d91c>] descend_children+0x30/0x6c
 [<1059e164>] device_for_each_child+0x60/0x98
 [<1019cd54>] parse_tree_node+0x40/0x54
 [<1019d86c>] check_parent+0xf0/0x170
 [<1019d91c>] descend_children+0x30/0x6c
 [<1059e164>] device_for_each_child+0x60/0x98
 [<1019d938>] descend_children+0x4c/0x6c
 [<1059e164>] device_for_each_child+0x60/0x98
 [<1019cd54>] parse_tree_node+0x40/0x54
 [<1019cffc>] hwpath_to_device+0xa4/0xc4

Signed-off-by: Helge Deller <deller@gmx.de>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/parisc/kernel/drivers.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/arch/parisc/kernel/drivers.c
+++ b/arch/parisc/kernel/drivers.c
@@ -648,6 +648,10 @@ static int match_pci_device(struct devic
 					(modpath->mod == PCI_FUNC(devfn)));
 	}
 
+	/* index might be out of bounds for bc[] */
+	if (index >= 6)
+		return 0;
+
 	id = PCI_SLOT(pdev->devfn) | (PCI_FUNC(pdev->devfn) << 5);
 	return (modpath->bc[index] == id);
 }

^ permalink raw reply

* Re: [PATCH v1] gso: fix marking TCP checksum flag in TCP segments
From: Ophir Munk @ 2018-04-22 14:47 UTC (permalink / raw)
  To: dev@dpdk.org, Jiayu Hu, Yigit, Ferruh
  Cc: Thomas Monjalon, Olga Shern, Pascal Mazon, stable@dpdk.org
In-Reply-To: <1524406859-29585-1-git-send-email-ophirmu@mellanox.com>

Hi Ferruh, Thomas,
Please note that this patch should be merged before the net/tap TSO patches:

https://dpdk.org/dev/patchwork/patch/38666/
https://dpdk.org/dev/patchwork/patch/38667/

Regards,
Ophir

> -----Original Message-----
> From: Ophir Munk
> Sent: Sunday, April 22, 2018 5:21 PM
> To: dev@dpdk.org; Jiayu Hu <jiayu.hu@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Olga Shern
> <olgas@mellanox.com>; Pascal Mazon <pascal.mazon@6wind.com>; Ophir
> Munk <ophirmu@mellanox.com>; stable@dpdk.org
> Subject: [PATCH v1] gso: fix marking TCP checksum flag in TCP segments
> 
> Large TCP packets which are marked with PKT_TX_TCP_SEG flag are
> segmented and the flag is cleared in the resulting segments, however, the
> segments checksum is not updated. It is therefore required to set the
> PKT_TX_TCP_CKSUM flag in each TCP segment in order to mark for the
> sending driver the need to update the TCP checksum before transmitting the
> segment.
> 
> Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
>  lib/librte_gso/rte_gso.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c index
> a44e3d4..e9ce9ce 100644
> --- a/lib/librte_gso/rte_gso.c
> +++ b/lib/librte_gso/rte_gso.c
> @@ -50,12 +50,14 @@ rte_gso_segment(struct rte_mbuf *pkt,
>  			((IS_IPV4_GRE_TCP4(pkt->ol_flags) &&
>  			 (gso_ctx->gso_types &
> DEV_TX_OFFLOAD_GRE_TNL_TSO)))) {
>  		pkt->ol_flags &= (~PKT_TX_TCP_SEG);
> +		pkt->ol_flags |= PKT_TX_TCP_CKSUM;
>  		ret = gso_tunnel_tcp4_segment(pkt, gso_size, ipid_delta,
>  				direct_pool, indirect_pool,
>  				pkts_out, nb_pkts_out);
>  	} else if (IS_IPV4_TCP(pkt->ol_flags) &&
>  			(gso_ctx->gso_types &
> DEV_TX_OFFLOAD_TCP_TSO)) {
>  		pkt->ol_flags &= (~PKT_TX_TCP_SEG);
> +		pkt->ol_flags |= PKT_TX_TCP_CKSUM;
>  		ret = gso_tcp4_segment(pkt, gso_size, ipid_delta,
>  				direct_pool, indirect_pool,
>  				pkts_out, nb_pkts_out);
> --
> 2.7.4

^ permalink raw reply

* [PATCH 4.4 22/97] ubifs: Check ubifs_wbuf_sync() return code
From: Greg Kroah-Hartman @ 2018-04-22 13:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Richard Weinberger
In-Reply-To: <20180422135304.577223025@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Richard Weinberger <richard@nod.at>

commit aac17948a7ce01fb60b9ee6cf902967a47b3ce26 upstream.

If ubifs_wbuf_sync() fails we must not write a master node with the
dirty marker cleared.
Otherwise it is possible that in case of an IO error while syncing we
mark the filesystem as clean and UBIFS refuses to recover upon next
mount.

Cc: <stable@vger.kernel.org>
Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
Signed-off-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/ubifs/super.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1728,8 +1728,11 @@ static void ubifs_remount_ro(struct ubif
 
 	dbg_save_space_info(c);
 
-	for (i = 0; i < c->jhead_cnt; i++)
-		ubifs_wbuf_sync(&c->jheads[i].wbuf);
+	for (i = 0; i < c->jhead_cnt; i++) {
+		err = ubifs_wbuf_sync(&c->jheads[i].wbuf);
+		if (err)
+			ubifs_ro_mode(c, err);
+	}
 
 	c->mst_node->flags &= ~cpu_to_le32(UBIFS_MST_DIRTY);
 	c->mst_node->flags |= cpu_to_le32(UBIFS_MST_NO_ORPHS);
@@ -1795,8 +1798,11 @@ static void ubifs_put_super(struct super
 			int err;
 
 			/* Synchronize write-buffers */
-			for (i = 0; i < c->jhead_cnt; i++)
-				ubifs_wbuf_sync(&c->jheads[i].wbuf);
+			for (i = 0; i < c->jhead_cnt; i++) {
+				err = ubifs_wbuf_sync(&c->jheads[i].wbuf);
+				if (err)
+					ubifs_ro_mode(c, err);
+			}
 
 			/*
 			 * We are being cleanly unmounted which means the

^ permalink raw reply

* Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary
From: Ramsay Jones @ 2018-04-22 14:45 UTC (permalink / raw)
  To: Duy Nguyen, git, gitster, philipoakley, sunshine, szeder.dev
In-Reply-To: <20180421165618.GA30287@duynguyen.home>



On 21/04/18 17:56, Duy Nguyen wrote:
> On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
>> Changes:
>>
>> - remove the deprecated column in command-list.txt. My change break it
>>   anyway if anyone uses it.
>> - fix up failed tests that I marked in the RFC and kinda forgot about it.
>> - fix bashisms in generate-cmdlist.sh
>> - fix segfaul in "git help"
> 
> Sorry I forgot the interdiff
> 
[snip]

> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
> index fd2a7f27dc..53208ab20e 100755
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -25,6 +25,15 @@ test_expect_success "setup" '
>  	EOF
>  '
>  
> +# make sure to exercise these code paths, the output is a bit tricky
> +# to verify
> +test_expect_success 'basic help commands' '
> +	git help >/dev/null &&
> +	git help -a >/dev/null &&
> +	git help -g >/dev/null &&
> +	git help -av >/dev/null
> +'
> +
I think you need to try a little harder than this! ;-)

This test would not have noticed the recent failure (what annoyed me was
that git build without error and then the test-suite sailed through with
nary a squeak of complaint). Viz:

  $ ./git help
  usage: git [--version] [--help] [-C <path>] [-c <name>=<value>]
             [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
             [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]
             [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
             <command> [<args>]
  
  These are common Git commands used in various situations:
  
  'git help -a' and 'git help -g' list available subcommands and some
  concept guides. See 'git help <command>' or 'git help <concept>'
  to read about a specific subcommand or concept.
  $ echo $?
  0
  $ 

  $ ./git help -g
  The common Git guides are:
  
  
  'git help -a' and 'git help -g' list available subcommands and some
  concept guides. See 'git help <command>' or 'git help <concept>'
  to read about a specific subcommand or concept.
  $ echo $?
  0
  $ 

  $ ./git help -av
  Main Porcelain Commands
  
  
  Ancillary Commands / Manipulators
  
  
  Ancillary Commands / Interrogators
  
  
  Interacting with Others
  
  
  Low-level Commands / Manipulators
  
  
  Low-level Commands / Interrogators
  
  
  Low-level Commands / Synching Repositories
  
  
  Low-level Commands / Internal Helpers
  
  $ echo $?
  0
  $ 
 
I started to add some tests, like so:

  diff --git a/t/t0012-help.sh b/t/t0012-help.sh
  index fd2a7f27d..7e10c2862 100755
  --- a/t/t0012-help.sh
  +++ b/t/t0012-help.sh
  @@ -49,6 +49,22 @@ test_expect_success "--help does not work for guides" "
          test_i18ncmp expect actual
   "
   
  +test_expect_success 'git help' '
  +       git help >help.output &&
  +       test_i18ngrep "^   clone  " help.output &&
  +       test_i18ngrep "^   add    " help.output &&
  +       test_i18ngrep "^   log    " help.output &&
  +       test_i18ngrep "^   commit " help.output &&
  +       test_i18ngrep "^   fetch  " help.output
  +'
  +
  +test_expect_success 'git help -g' '
  +       git help -g >help.output &&
  +       test_i18ngrep "^   attributes " help.output &&
  +       test_i18ngrep "^   everyday   " help.output &&
  +       test_i18ngrep "^   tutorial   " help.output
  +'
  +
   test_expect_success 'generate builtin list' '
          git --list-cmds=builtins >builtins
   '

These tests, while not rigorous, did at least correctly catch the bad
build for me. I was about to add the obvious one for 'git help -av',
when I decided to catch the problem 'at source', viz:

  diff --git a/help.c b/help.c
  index a47f7e2c4..13790bb54 100644
  --- a/help.c
  +++ b/help.c
  @@ -196,6 +196,9 @@ static void extract_common_cmds(struct cmdname_help **p_common_cmds,
   	int i, nr = 0;
   	struct cmdname_help *common_cmds;
   
  +	if (ARRAY_SIZE(command_list) == 0)
  +		BUG("empty command_list");
  +
   	ALLOC_ARRAY(common_cmds, ARRAY_SIZE(command_list));
   
   	for (i = 0; i < ARRAY_SIZE(command_list); i++) {
  @@ -279,6 +282,9 @@ void list_porcelain_cmds(void)
   	int i, nr = ARRAY_SIZE(command_list);
   	struct cmdname_help *cmds = command_list;
   
  +	if (nr == 0)
  +		BUG("empty command_list");
  +
   	for (i = 0; i < nr; i++) {
   		if (cmds[i].category != CAT_mainporcelain)
   			continue;
  @@ -321,6 +327,9 @@ void list_common_guides_help(void)
   	int nr = ARRAY_SIZE(command_list);
   	struct cmdname_help *cmds = command_list;
   
  +	if (nr == 0)
  +		BUG("empty command_list");
  +
   	QSORT(cmds, nr, cmd_category_cmp);
   
   	for (i = 0; i < nr; i++) {
  @@ -343,6 +352,9 @@ void list_all_cmds_help(void)
   	int nr = ARRAY_SIZE(command_list);
   	struct cmdname_help *cmds = command_list;
   
  +	if (nr == 0)
  +		BUG("empty command_list");
  +
   	for (i = 0; i < nr; i++) {
   		struct cmdname_help *cmd = cmds + i;
   
  
This had a very dramatic effect on the test-suite, since every single
test file failed while sourcing 'test-lib.sh'. [The test for having
built git ('"$GIT_BUILD_DIR/git" >/dev/null') tries to output help,
because you haven't given a command, and hits BUG - core dump!]

I haven't tried this patch series yet (I will hopefully find some
time tonight), but it looks like it will fix the problem.

ATB,
Ramsay Jones


^ permalink raw reply

* TCP GSO support above IPv6
From: Ophir Munk @ 2018-04-22 14:45 UTC (permalink / raw)
  To: dev@dpdk.org, Jiayu Hu, Mark Kavanagh; +Cc: Thomas Monjalon, Olga Shern

Hello Jiayu,
I wonder why librte_gso only supports TCP GSO above IPv4. 
Can you please explain why TCP GSO is not supported above IPv6? 
Are there plans to support it?

Regards,
Ophir

^ permalink raw reply

* [PATCH 4.4 42/97] powerpc/powernv: Fix OPAL NVRAM driver OPAL_BUSY loops
From: Greg Kroah-Hartman @ 2018-04-22 13:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Nicholas Piggin, Michael Ellerman
In-Reply-To: <20180422135304.577223025@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Nicholas Piggin <npiggin@gmail.com>

commit 3b8070335f751aac9f1526ae2e012e6f5b8b0f21 upstream.

The OPAL NVRAM driver does not sleep in case it gets OPAL_BUSY or
OPAL_BUSY_EVENT from firmware, which causes large scheduling
latencies, and various lockup errors to trigger (again, BMC reboot
can cause it).

Fix this by converting it to the standard form OPAL_BUSY loop that
sleeps.

Fixes: 628daa8d5abf ("powerpc/powernv: Add RTC and NVRAM support plus RTAS fallbacks")
Depends-on: 34dd25de9fe3 ("powerpc/powernv: define a standard delay for OPAL_BUSY type retry loops")
Cc: stable@vger.kernel.org # v3.2+
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/powerpc/platforms/powernv/opal-nvram.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- a/arch/powerpc/platforms/powernv/opal-nvram.c
+++ b/arch/powerpc/platforms/powernv/opal-nvram.c
@@ -11,6 +11,7 @@
 
 #define DEBUG
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of.h>
@@ -56,8 +57,12 @@ static ssize_t opal_nvram_write(char *bu
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_write_nvram(__pa(buf), count, off);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 
 	if (rc)

^ permalink raw reply

* [PATCH 4.4 44/97] HID: core: Fix size as type u32
From: Greg Kroah-Hartman @ 2018-04-22 13:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Aaron Ma, Jiri Kosina
In-Reply-To: <20180422135304.577223025@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Aaron Ma <aaron.ma@canonical.com>

commit 6de0b13cc0b4ba10e98a9263d7a83b940720b77a upstream.

When size is negative, calling memset will make segment fault.
Declare the size as type u32 to keep memset safe.

size in struct hid_report is unsigned, fix return type of
hid_report_len to u32.

Cc: stable@vger.kernel.org
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/hid/hid-core.c |   10 +++++-----
 include/linux/hid.h    |    6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1331,7 +1331,7 @@ u8 *hid_alloc_report_buf(struct hid_repo
 	 * of implement() working on 8 byte chunks
 	 */
 
-	int len = hid_report_len(report) + 7;
+	u32 len = hid_report_len(report) + 7;
 
 	return kmalloc(len, flags);
 }
@@ -1396,7 +1396,7 @@ void __hid_request(struct hid_device *hi
 {
 	char *buf;
 	int ret;
-	int len;
+	u32 len;
 
 	buf = hid_alloc_report_buf(report, GFP_KERNEL);
 	if (!buf)
@@ -1422,14 +1422,14 @@ out:
 }
 EXPORT_SYMBOL_GPL(__hid_request);
 
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
 		int interrupt)
 {
 	struct hid_report_enum *report_enum = hid->report_enum + type;
 	struct hid_report *report;
 	struct hid_driver *hdrv;
 	unsigned int a;
-	int rsize, csize = size;
+	u32 rsize, csize = size;
 	u8 *cdata = data;
 	int ret = 0;
 
@@ -1487,7 +1487,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
  *
  * This is data entry for lower layers.
  */
-int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int interrupt)
+int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt)
 {
 	struct hid_report_enum *report_enum;
 	struct hid_driver *hdrv;
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -793,7 +793,7 @@ extern int hidinput_connect(struct hid_d
 extern void hidinput_disconnect(struct hid_device *);
 
 int hid_set_field(struct hid_field *, unsigned, __s32);
-int hid_input_report(struct hid_device *, int type, u8 *, int, int);
+int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
 int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
 struct hid_field *hidinput_get_led_field(struct hid_device *hid);
 unsigned int hidinput_count_leds(struct hid_device *hid);
@@ -1098,13 +1098,13 @@ static inline void hid_hw_wait(struct hi
  *
  * @report: the report we want to know the length
  */
-static inline int hid_report_len(struct hid_report *report)
+static inline u32 hid_report_len(struct hid_report *report)
 {
 	/* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
 	return ((report->size - 1) >> 3) + 1 + (report->id > 0);
 }
 
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
 		int interrupt);
 
 /* HID quirks API */

^ permalink raw reply

* [PATCH 4.4 45/97] ASoC: ssm2602: Replace reg_default_raw with reg_default
From: Greg Kroah-Hartman @ 2018-04-22 13:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, James Kelly, Mark Brown
In-Reply-To: <20180422135304.577223025@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: James Kelly <jamespeterkelly@gmail.com>

commit a01df75ce737951ad13a08d101306e88c3f57cb2 upstream.

SSM2602 driver is broken on recent kernels (at least
since 4.9). User space applications such as amixer or
alsamixer get EIO when attempting to access codec
controls via the relevant IOCTLs.

Root cause of these failures is the regcache_hw_init
function in drivers/base/regmap/regcache.c, which
prevents regmap cache initalization from the
reg_defaults_raw element of the regmap_config structure
when registers are write only. It also disables the
regmap cache entirely when all registers are write only
or volatile as is the case for the SSM2602 driver.

Using the reg_defaults element of the regmap_config
structure rather than the reg_defaults_raw element to
initalize the regmap cache avoids the logic in the
regcache_hw_init function entirely. It also makes this
driver consistent with other ASoC codec drivers, as
this driver was the ONLY codec driver that used the
reg_defaults_raw element to initalize the cache.

Tested on Digilent Zybo Z7 development board which has
a SSM2603 codec chip connected to a Xilinx Zynq SoC.

Signed-off-by: James Kelly <jamespeterkelly@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 sound/soc/codecs/ssm2602.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

--- a/sound/soc/codecs/ssm2602.c
+++ b/sound/soc/codecs/ssm2602.c
@@ -54,10 +54,17 @@ struct ssm2602_priv {
  * using 2 wire for device control, so we cache them instead.
  * There is no point in caching the reset register
  */
-static const u16 ssm2602_reg[SSM2602_CACHEREGNUM] = {
-	0x0097, 0x0097, 0x0079, 0x0079,
-	0x000a, 0x0008, 0x009f, 0x000a,
-	0x0000, 0x0000
+static const struct reg_default ssm2602_reg[SSM2602_CACHEREGNUM] = {
+	{ .reg = 0x00, .def = 0x0097 },
+	{ .reg = 0x01, .def = 0x0097 },
+	{ .reg = 0x02, .def = 0x0079 },
+	{ .reg = 0x03, .def = 0x0079 },
+	{ .reg = 0x04, .def = 0x000a },
+	{ .reg = 0x05, .def = 0x0008 },
+	{ .reg = 0x06, .def = 0x009f },
+	{ .reg = 0x07, .def = 0x000a },
+	{ .reg = 0x08, .def = 0x0000 },
+	{ .reg = 0x09, .def = 0x0000 }
 };
 
 
@@ -618,8 +625,8 @@ const struct regmap_config ssm2602_regma
 	.volatile_reg = ssm2602_register_volatile,
 
 	.cache_type = REGCACHE_RBTREE,
-	.reg_defaults_raw = ssm2602_reg,
-	.num_reg_defaults_raw = ARRAY_SIZE(ssm2602_reg),
+	.reg_defaults = ssm2602_reg,
+	.num_reg_defaults = ARRAY_SIZE(ssm2602_reg),
 };
 EXPORT_SYMBOL_GPL(ssm2602_regmap_config);
 

^ permalink raw reply

* [PATCH 4.4 48/97] jbd2: if the journal is aborted then dont allow update of the log tail
From: Greg Kroah-Hartman @ 2018-04-22 13:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Theodore Tso
In-Reply-To: <20180422135304.577223025@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Theodore Ts'o <tytso@mit.edu>

commit 85e0c4e89c1b864e763c4e3bb15d0b6d501ad5d9 upstream.

This updates the jbd2 superblock unnecessarily, and on an abort we
shouldn't truncate the log.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/jbd2/journal.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -914,7 +914,7 @@ out:
 }
 
 /*
- * This is a variaon of __jbd2_update_log_tail which checks for validity of
+ * This is a variation of __jbd2_update_log_tail which checks for validity of
  * provided log tail and locks j_checkpoint_mutex. So it is safe against races
  * with other threads updating log tail.
  */
@@ -1384,6 +1384,9 @@ int jbd2_journal_update_sb_log_tail(jour
 	journal_superblock_t *sb = journal->j_superblock;
 	int ret;
 
+	if (is_journal_aborted(journal))
+		return -EIO;
+
 	BUG_ON(!mutex_is_locked(&journal->j_checkpoint_mutex));
 	jbd_debug(1, "JBD2: updating superblock (start %lu, seq %u)\n",
 		  tail_block, tail_tid);

^ permalink raw reply

* Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel
From: David Julian Veenstra @ 2018-04-22 14:41 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: devel, devicetree, lars, Michael.Hennerich, linux-iio, robh+dt,
	pmeerw, knaack.h, daniel.baluta
In-Reply-To: <20180421180843.3bd3042a@archlinux>


On 21, April 2018 19:08, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:37 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The iio resolver drivers in staging use angle channels. This patch
>> add missing documentation for this type of channel.
>> 
>> As was discussed in [1], radians is chosen as the unit, to match the
>> unit of angular velocity.
>> 
>> [1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>> Change in v2:
>>   - Introduces in this version.
>> 
>>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 6a5f34b4d5b9..8ad0e55f99ee 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -190,6 +190,15 @@ Description:
>>  		but should match other such assignments on device).
>>  		Units after application of scale and offset are m/s^2.
>>  
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
> This surprised me.  A resolver is not going to inherently have any
> notion of a particular axis.
> Would expect.
> in_angl_raw

For the attributes that were added, I tried to match it with
the angular velocity counterpart. But it indeed doesn't make
much sense to have an axis for a resolver. I'll remove axis
modifier for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> +KernelVersion:	4.17
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Angle about axis x, y or z (may be arbitrarily assigned). Units
>> +		after application of scale and offset are radians.
>> +
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
>> @@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:
>> @@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:

^ permalink raw reply

* Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel
From: David Julian Veenstra @ 2018-04-22 14:41 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, pmeerw, robh+dt, Michael.Hennerich, knaack.h, daniel.baluta,
	linux-iio, devel, devicetree
In-Reply-To: <20180421180843.3bd3042a@archlinux>


On 21, April 2018 19:08, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:37 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The iio resolver drivers in staging use angle channels. This patch
>> add missing documentation for this type of channel.
>> 
>> As was discussed in [1], radians is chosen as the unit, to match the
>> unit of angular velocity.
>> 
>> [1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>> Change in v2:
>>   - Introduces in this version.
>> 
>>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 6a5f34b4d5b9..8ad0e55f99ee 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -190,6 +190,15 @@ Description:
>>  		but should match other such assignments on device).
>>  		Units after application of scale and offset are m/s^2.
>>  
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
> This surprised me.  A resolver is not going to inherently have any
> notion of a particular axis.
> Would expect.
> in_angl_raw

For the attributes that were added, I tried to match it with
the angular velocity counterpart. But it indeed doesn't make
much sense to have an axis for a resolver. I'll remove axis
modifier for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> +KernelVersion:	4.17
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Angle about axis x, y or z (may be arbitrarily assigned). Units
>> +		after application of scale and offset are radians.
>> +
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
>> @@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:
>> @@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx6qdl-cubox-i: Move card-detect GPIO to 1.5 SOM devices only
From: Russell King - ARM Linux @ 2018-04-22 14:39 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-arm-kernel, devicetree, linux-kernel, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland
In-Reply-To: <20180422142151.17910-1-contact@paulk.fr>

On Sun, Apr 22, 2018 at 04:21:51PM +0200, Paul Kocialkowski wrote:
> The Solid-Run CuBox-i lower board used in the first generation of
> CuBox-i devices feature a hinged micro SD card slot, that does not have
> card-detect capability. Since the card-detect GPIO was specified in the
> common cubox-i dtsi, it is moved to each device using the 1.5 SOM and is
> thus removed from the imx6q-cubox-i dts.

This seems incorrect.

The 1.5 SOM is a relatively recent thing, and I have one of the first
Cubox-i's that were produced which is not hinged, and does not have a
1.5 SOM.  There is _no_ correlation between the 1.5 SOM and the SD
card slot.

Talking to folk at SolidRun, we're all confused about your assertions.

We're also confused about "hinged micro SD card slot" - Cubox-i's
come with either a push-push slot or a static (push-in, pull-out) slot.
(Prototypes were the latter.)

Maybe someone's modified yours?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* [PATCH] ARM: dts: imx6qdl-cubox-i: Move card-detect GPIO to 1.5 SOM devices only
From: Russell King - ARM Linux @ 2018-04-22 14:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180422142151.17910-1-contact@paulk.fr>

On Sun, Apr 22, 2018 at 04:21:51PM +0200, Paul Kocialkowski wrote:
> The Solid-Run CuBox-i lower board used in the first generation of
> CuBox-i devices feature a hinged micro SD card slot, that does not have
> card-detect capability. Since the card-detect GPIO was specified in the
> common cubox-i dtsi, it is moved to each device using the 1.5 SOM and is
> thus removed from the imx6q-cubox-i dts.

This seems incorrect.

The 1.5 SOM is a relatively recent thing, and I have one of the first
Cubox-i's that were produced which is not hinged, and does not have a
1.5 SOM.  There is _no_ correlation between the 1.5 SOM and the SD
card slot.

Talking to folk at SolidRun, we're all confused about your assertions.

We're also confused about "hinged micro SD card slot" - Cubox-i's
come with either a push-push slot or a static (push-in, pull-out) slot.
(Prototypes were the latter.)

Maybe someone's modified yours?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* [Buildroot] [PATCH v2] configs: add openssl for kernel where necessary
From: Vincent Stehle @ 2018-04-22 14:38 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <789c3248-327e-2c69-208d-c88bc72363b2@grinn-global.com>

From: Vincent Stehl? <vincent.stehle@laposte.net>

Add host-openssl to those configs, which need it for the Linux kernel build.

Signed-off-by: Vincent Stehl? <vincent.stehle@laposte.net>
Cc: Matthew Weber <matthew.weber@rockwellcollins.com>
Cc: Marcin Niestroj <m.niestroj@grinn-global.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>

---

On Thu, Apr 19, 2018 at 10:14:24AM +0200, Marcin Niestroj wrote:
> Thanks for noticing grinn_chiliboard_defconfig! Please also add
> grinn_liteboard_defconfig, as it has the same build issues with
> missing openssl.

Hi Marcin,

Here you go!

I also updated mx6cubox_defconfig while at it.

Best regards,

Vincent.


 configs/freescale_t1040d4rdb_defconfig | 1 +
 configs/grinn_chiliboard_defconfig     | 1 +
 configs/grinn_liteboard_defconfig      | 1 +
 configs/mx25pdk_defconfig              | 1 +
 configs/mx6cubox_defconfig             | 1 +
 configs/wandboard_defconfig            | 1 +
 configs/warp7_defconfig                | 1 +
 7 files changed, 7 insertions(+)

diff --git a/configs/freescale_t1040d4rdb_defconfig b/configs/freescale_t1040d4rdb_defconfig
index b8d162736c..df44046ef5 100644
--- a/configs/freescale_t1040d4rdb_defconfig
+++ b/configs/freescale_t1040d4rdb_defconfig
@@ -15,6 +15,7 @@ BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.15.7"
 BR2_LINUX_KERNEL_DEFCONFIG="corenet64_smp"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="fsl/t1040d4rdb"
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
 
 # Filesystem
 BR2_TARGET_ROOTFS_CPIO=y
diff --git a/configs/grinn_chiliboard_defconfig b/configs/grinn_chiliboard_defconfig
index 07fa0b3797..81d7bae911 100644
--- a/configs/grinn_chiliboard_defconfig
+++ b/configs/grinn_chiliboard_defconfig
@@ -10,6 +10,7 @@ BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.16.1"
 BR2_LINUX_KERNEL_DEFCONFIG="omap2plus"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="am335x-chiliboard"
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_UBOOT=y
diff --git a/configs/grinn_liteboard_defconfig b/configs/grinn_liteboard_defconfig
index f644d36f06..5f3ca07552 100644
--- a/configs/grinn_liteboard_defconfig
+++ b/configs/grinn_liteboard_defconfig
@@ -10,6 +10,7 @@ BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.16.2"
 BR2_LINUX_KERNEL_DEFCONFIG="imx_v6_v7"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx6ul-liteboard"
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_UBOOT=y
diff --git a/configs/mx25pdk_defconfig b/configs/mx25pdk_defconfig
index f86ce7af02..ec4f8b8289 100644
--- a/configs/mx25pdk_defconfig
+++ b/configs/mx25pdk_defconfig
@@ -32,3 +32,4 @@ BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.15.7"
 BR2_LINUX_KERNEL_DEFCONFIG="imx_v4_v5"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx25-pdk"
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
diff --git a/configs/mx6cubox_defconfig b/configs/mx6cubox_defconfig
index 6357720aa4..2996c375ff 100644
--- a/configs/mx6cubox_defconfig
+++ b/configs/mx6cubox_defconfig
@@ -24,6 +24,7 @@ BR2_LINUX_KERNEL_DEFCONFIG="imx_v6_v7"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx6q-cubox-i imx6dl-cubox-i imx6q-hummingboard imx6dl-hummingboard"
 BR2_LINUX_KERNEL_INSTALL_TARGET=y
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
 # required tools to create the SD card image
 BR2_PACKAGE_HOST_DOSFSTOOLS=y
 BR2_PACKAGE_HOST_GENIMAGE=y
diff --git a/configs/wandboard_defconfig b/configs/wandboard_defconfig
index 406bd113ff..4265fdcfb0 100644
--- a/configs/wandboard_defconfig
+++ b/configs/wandboard_defconfig
@@ -24,6 +24,7 @@ BR2_LINUX_KERNEL_DEFCONFIG="imx_v6_v7"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx6q-wandboard imx6dl-wandboard imx6q-wandboard-revb1 imx6dl-wandboard-revb1 imx6q-wandboard-revd1 imx6dl-wandboard-revd1 imx6qp-wandboard-revd1"
 BR2_LINUX_KERNEL_INSTALL_TARGET=y
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
 # required tools to create the SD card image
 BR2_PACKAGE_HOST_DOSFSTOOLS=y
 BR2_PACKAGE_HOST_GENIMAGE=y
diff --git a/configs/warp7_defconfig b/configs/warp7_defconfig
index 344c4430b0..e1744d8275 100644
--- a/configs/warp7_defconfig
+++ b/configs/warp7_defconfig
@@ -19,6 +19,7 @@ BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.15.15"
 BR2_LINUX_KERNEL_DEFCONFIG="imx_v6_v7"
 BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx7s-warp"
+BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
 
 # U-Boot
 BR2_TARGET_UBOOT=y
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel
From: David Julian Veenstra @ 2018-04-22 14:37 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: devel, devicetree, lars, Michael.Hennerich, linux-iio, robh+dt,
	pmeerw, knaack.h, daniel.baluta
In-Reply-To: <20180421180704.2b5b183b@archlinux>


On 21, April 2018 19:07, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:09 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The sysfs iio ABI states radians per second is expected as the unit for
>> angular velocity, but the 12-bit angular velocity register has rps
>> as its unit. So a fractional scaling factor of approximately 2 * Pi is
>> added to the angular velocity channel.
>> 
>> The added comments will also be relevant for the scaling factor of
>> the angle channel.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> Comment inline.  The function you are talking about isn't used
> in the majority of likely use cases for this part.  The maths will actually
> be done in userspace (which can use floating point).
>
> Thanks,
>
> Jonathan
>
>> ---
>> Changes in v2:
>>   - Move explanation of Pi approximation to top of switch statement,
>>     as this will also be relevant to angle channel.
>>   - Replaced 33102 / 2 with 16551 on line 84.
>> 
>>  drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
>>  1 file changed, 59 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 29a9bb666e7b..6c56257be3b1 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	int ret = 0;
>>  	u16 vel;
>>  
>> -	mutex_lock(&st->lock);
>> -	gpiod_set_value(st->sample, 0);
>> +	/*
>> +	 * Below a fractional approximation of Pi is needed.
>> +	 * The following approximation will be used: 103993 / 33102.
>> +	 * This is accurate in 9 decimals places.
>> +	 *
>> +	 * This fraction is based on OEIS series of nominator/denominator
>> +	 * of convergents to Pi (A002485 and A002486).
>> +	 */
>> +	switch (m) {
>> +	case IIO_CHAN_INFO_SCALE:
>> +		switch (chan->type) {
>> +		case IIO_ANGL_VEL:
>> +			/*
>> +			 * 2 * Pi ~= 2 * 103993 / 33102
>> +			 *
>> +			 * iio_convert_raw_to_processed uses integer
>> +			 * division. This will cause at most 5% error
>> +			 * (for very small values). But for 99.5% of the values
>> +			 * it will cause less that 1% error.
> This is an interesting comment, but relies on anyone actually
> using iio_convert_raw_to_processed with this device.
>
> I would imagine that 'in kernel' users of a resolver (who would use
> that function) will be few and far between.  Mostly this will just
> get passed to userspace.  That involves this being converted to
> a decimal.  I would just specify it as one in the first place.

Hmm, I didn't realize that it might never be used in kernel. A
decimal representation is indeed a lot more clear. I'll change
scale value type to IIO_VAL_INT_PLUS_MICRO for both the angular
velocity and angel channel.

Best regards,
David Veenstra

>
>> +			 */
>> +			*val = 103993;
>> +			*val2 = 16551;
>> +			return IIO_VAL_FRACTIONAL;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&st->lock);
>> +		gpiod_set_value(st->sample, 0);
>> +
>> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> +		udelay(1);
>> +		gpiod_set_value(st->sample, 1);
>> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +
>> +		ret = spi_read(st->sdev, st->rx, 2);
>> +		if (ret < 0) {
>> +			mutex_unlock(&st->lock);
>> +			return ret;
>> +		}
>>  
>> -	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> -	udelay(1);
>> -	gpiod_set_value(st->sample, 1);
>> -	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +		vel = be16_to_cpup((__be16 *)st->rx);
>> +		switch (chan->type) {
>> +		case IIO_ANGL:
>> +			*val = vel >> 4;
>> +			break;
>> +		case IIO_ANGL_VEL:
>> +			*val = sign_extend32((s16)vel >> 4, 11);
>> +			break;
>> +		default:
>> +			mutex_unlock(&st->lock);
>> +			return -EINVAL;
>> +		}
>>  
>> -	ret = spi_read(st->sdev, st->rx, 2);
>> -	if (ret < 0) {
>> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> +		udelay(1);
>>  		mutex_unlock(&st->lock);
>> -		return ret;
>> -	}
>>  
>> -	vel = be16_to_cpup((__be16 *)st->rx);
>> -	switch (chan->type) {
>> -	case IIO_ANGL:
>> -		*val = vel >> 4;
>> -		break;
>> -	case IIO_ANGL_VEL:
>> -		*val = sign_extend32((s16)vel >> 4, 11);
>> -		break;
>> +		return IIO_VAL_INT;
>>  	default:
>> -		mutex_unlock(&st->lock);
>> -		return -EINVAL;
>> +		break;
>>  	}
>>  
>> -	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> -	udelay(1);
>> -	mutex_unlock(&st->lock);
>> -
>> -	return IIO_VAL_INT;
>> +	return -EINVAL;
>>  }
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {
>> @@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>>  		.indexed = 1,
>>  		.channel = 0,
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>>  	}
>>  };
>>  

^ permalink raw reply

* Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel
From: David Julian Veenstra @ 2018-04-22 14:37 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, pmeerw, robh+dt, Michael.Hennerich, knaack.h, daniel.baluta,
	linux-iio, devel, devicetree
In-Reply-To: <20180421180704.2b5b183b@archlinux>


On 21, April 2018 19:07, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:09 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The sysfs iio ABI states radians per second is expected as the unit for
>> angular velocity, but the 12-bit angular velocity register has rps
>> as its unit. So a fractional scaling factor of approximately 2 * Pi is
>> added to the angular velocity channel.
>> 
>> The added comments will also be relevant for the scaling factor of
>> the angle channel.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> Comment inline.  The function you are talking about isn't used
> in the majority of likely use cases for this part.  The maths will actually
> be done in userspace (which can use floating point).
>
> Thanks,
>
> Jonathan
>
>> ---
>> Changes in v2:
>>   - Move explanation of Pi approximation to top of switch statement,
>>     as this will also be relevant to angle channel.
>>   - Replaced 33102 / 2 with 16551 on line 84.
>> 
>>  drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
>>  1 file changed, 59 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 29a9bb666e7b..6c56257be3b1 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	int ret = 0;
>>  	u16 vel;
>>  
>> -	mutex_lock(&st->lock);
>> -	gpiod_set_value(st->sample, 0);
>> +	/*
>> +	 * Below a fractional approximation of Pi is needed.
>> +	 * The following approximation will be used: 103993 / 33102.
>> +	 * This is accurate in 9 decimals places.
>> +	 *
>> +	 * This fraction is based on OEIS series of nominator/denominator
>> +	 * of convergents to Pi (A002485 and A002486).
>> +	 */
>> +	switch (m) {
>> +	case IIO_CHAN_INFO_SCALE:
>> +		switch (chan->type) {
>> +		case IIO_ANGL_VEL:
>> +			/*
>> +			 * 2 * Pi ~= 2 * 103993 / 33102
>> +			 *
>> +			 * iio_convert_raw_to_processed uses integer
>> +			 * division. This will cause at most 5% error
>> +			 * (for very small values). But for 99.5% of the values
>> +			 * it will cause less that 1% error.
> This is an interesting comment, but relies on anyone actually
> using iio_convert_raw_to_processed with this device.
>
> I would imagine that 'in kernel' users of a resolver (who would use
> that function) will be few and far between.  Mostly this will just
> get passed to userspace.  That involves this being converted to
> a decimal.  I would just specify it as one in the first place.

Hmm, I didn't realize that it might never be used in kernel. A
decimal representation is indeed a lot more clear. I'll change
scale value type to IIO_VAL_INT_PLUS_MICRO for both the angular
velocity and angel channel.

Best regards,
David Veenstra

>
>> +			 */
>> +			*val = 103993;
>> +			*val2 = 16551;
>> +			return IIO_VAL_FRACTIONAL;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&st->lock);
>> +		gpiod_set_value(st->sample, 0);
>> +
>> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> +		udelay(1);
>> +		gpiod_set_value(st->sample, 1);
>> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +
>> +		ret = spi_read(st->sdev, st->rx, 2);
>> +		if (ret < 0) {
>> +			mutex_unlock(&st->lock);
>> +			return ret;
>> +		}
>>  
>> -	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> -	udelay(1);
>> -	gpiod_set_value(st->sample, 1);
>> -	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +		vel = be16_to_cpup((__be16 *)st->rx);
>> +		switch (chan->type) {
>> +		case IIO_ANGL:
>> +			*val = vel >> 4;
>> +			break;
>> +		case IIO_ANGL_VEL:
>> +			*val = sign_extend32((s16)vel >> 4, 11);
>> +			break;
>> +		default:
>> +			mutex_unlock(&st->lock);
>> +			return -EINVAL;
>> +		}
>>  
>> -	ret = spi_read(st->sdev, st->rx, 2);
>> -	if (ret < 0) {
>> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> +		udelay(1);
>>  		mutex_unlock(&st->lock);
>> -		return ret;
>> -	}
>>  
>> -	vel = be16_to_cpup((__be16 *)st->rx);
>> -	switch (chan->type) {
>> -	case IIO_ANGL:
>> -		*val = vel >> 4;
>> -		break;
>> -	case IIO_ANGL_VEL:
>> -		*val = sign_extend32((s16)vel >> 4, 11);
>> -		break;
>> +		return IIO_VAL_INT;
>>  	default:
>> -		mutex_unlock(&st->lock);
>> -		return -EINVAL;
>> +		break;
>>  	}
>>  
>> -	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> -	udelay(1);
>> -	mutex_unlock(&st->lock);
>> -
>> -	return IIO_VAL_INT;
>> +	return -EINVAL;
>>  }
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {
>> @@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>>  		.indexed = 1,
>>  		.channel = 0,
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>>  	}
>>  };
>>  

^ permalink raw reply

* Re: [PATCH v8 09/16] rebase: introduce the --rebase-merges option
From: Philip Oakley @ 2018-04-22 14:37 UTC (permalink / raw)
  To: Johannes Schindelin, git
  Cc: Junio C Hamano, Jacob Keller, Stefan Beller, Eric Sunshine,
	Phillip Wood, Igor Djordjevic, Johannes Sixt, Sergey Organov,
	Martin Ågren
In-Reply-To: <0c92bdd1829328544269722cbbd3edcd169bb148.1524306547.git.johannes.schindelin@gmx.de>

From: "Johannes Schindelin" <johannes.schindelin@gmx.de>
> Once upon a time, this here developer thought: wouldn't it be nice if,
> say, Git for Windows' patches on top of core Git could be represented as
> a thicket of branches, and be rebased on top of core Git in order to
> maintain a cherry-pick'able set of patch series?
>
> The original attempt to answer this was: git rebase --preserve-merges.
>
> However, that experiment was never intended as an interactive option,
> and it only piggy-backed on git rebase --interactive because that
> command's implementation looked already very, very familiar: it was
> designed by the same person who designed --preserve-merges: yours truly.
>
> Some time later, some other developer (I am looking at you, Andreas!
> ;-)) decided that it would be a good idea to allow --preserve-merges to
> be combined with --interactive (with caveats!) and the Git maintainer
> (well, the interim Git maintainer during Junio's absence, that is)
> agreed, and that is when the glamor of the --preserve-merges design
> started to fall apart rather quickly and unglamorously.
>
> The reason? In --preserve-merges mode, the parents of a merge commit (or
> for that matter, of *any* commit) were not stated explicitly, but were
> *implied* by the commit name passed to the `pick` command.
>
Aside: I think this para should be extracted to the --preserve-merges 
documentation to highlight what it does / why it is 'wrong' (not what would 
be expected in some case). It may also need to discuss the (figurative) 
Cousins vs. Siblings distinction [merge of branches external, or internal, 
to the rebase.

"In --preserve-merges, the commit being selected for merging is implied by 
the commit name  passed to the `pick` command (i.e. of the original merge 
commit), not that of the rebased version of that parent."

A similar issue occurs with (figuratively) '--ancestry-path --first parent' 
searches which lacks the alternate '--lead parent' post-walk selection. [1]. 
I don't think there is a dot notation to select the merge cousins, nor merge 
siblings either A.,B ? (that's dot-comma ;-)

> This made it impossible, for example, to reorder commits. Not to mention
> to flatten the branch topology or, deity forbid, to split topic branches
> into two.
>
> Alas, these shortcomings also prevented that mode (whose original
> purpose was to serve Git for Windows' needs, with the additional hope
> that it may be useful to others, too) from serving Git for Windows'
> needs.
>
> Five years later, when it became really untenable to have one unwieldy,
> big hodge-podge patch series of partly related, partly unrelated patches
> in Git for Windows that was rebased onto core Git's tags from time to
> time (earning the undeserved wrath of the developer of the ill-fated
> git-remote-hg series that first obsoleted Git for Windows' competing
> approach, only to be abandoned without maintainer later) was really
> untenable, the "Git garden shears" were born [*1*/*2*]: a script,
> piggy-backing on top of the interactive rebase, that would first
> determine the branch topology of the patches to be rebased, create a
> pseudo todo list for further editing, transform the result into a real
> todo list (making heavy use of the `exec` command to "implement" the
> missing todo list commands) and finally recreate the patch series on
> top of the new base commit.
>
> That was in 2013. And it took about three weeks to come up with the
> design and implement it as an out-of-tree script. Needless to say, the
> implementation needed quite a few years to stabilize, all the while the
> design itself proved itself sound.
>
> With this patch, the goodness of the Git garden shears comes to `git
> rebase -i` itself. Passing the `--rebase-merges` option will generate
> a todo list that can be understood readily, and where it is obvious
> how to reorder commits. New branches can be introduced by inserting
> `label` commands and calling `merge <label>`. And once this mode will
> have become stable and universally accepted, we can deprecate the design
> mistake that was `--preserve-merges`.
>
> Link *1*:
> https://github.com/msysgit/msysgit/blob/master/share/msysGit/shears.sh
> Link *2*:
> https://github.com/git-for-windows/build-extra/blob/master/shears.sh
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Documentation/git-rebase.txt           |  20 ++-
> contrib/completion/git-completion.bash |   2 +-
> git-rebase--interactive.sh             |   1 +
> git-rebase.sh                          |   6 +
> t/t3430-rebase-merges.sh               | 179 +++++++++++++++++++++++++
> 5 files changed, 206 insertions(+), 2 deletions(-)
> create mode 100755 t/t3430-rebase-merges.sh
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 3277ca14327..34e0f6a69c1 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -378,6 +378,23 @@ The commit list format can be changed by setting the 
> configuration option
> rebase.instructionFormat.  A customized instruction format will 
> automatically
> have the long commit hash prepended to the format.
>
> +-r::
> +--rebase-merges::
> + By default, a rebase will simply drop merge commits and only rebase
> + the non-merge commits. With this option, it will try to preserve
> + the branching structure within the commits that are to be rebased,
> + by recreating the merge commits. If a merge commit resolved any merge
> + or contained manual amendments, then they will have to be re-applied
> + manually.
> ++
> +This mode is similar in spirit to `--preserve-merges`, but in contrast to
> +that option works well in interactive rebases: commits can be reordered,
> +inserted and dropped at will.
> ++
> +It is currently only possible to recreate the merge commits using the
> +`recursive` merge strategy; Different merge strategies can be used only 
> via
> +explicit `exec git merge -s <strategy> [...]` commands.
> +
> -p::
> --preserve-merges::
>  Recreate merge commits instead of flattening the history by replaying
> @@ -780,7 +797,8 @@ BUGS
> The todo list presented by `--preserve-merges --interactive` does not
> represent the topology of the revision graph.  Editing commits and
> rewording their commit messages should work fine, but attempts to
> -reorder commits tend to produce counterintuitive results.
> +reorder commits tend to produce counterintuitive results. Use
> +`--rebase-merges` in such scenarios instead.
>
> For example, an attempt to rearrange
> ------------
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index a7570739454..d4c0a995c39 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1949,7 +1949,7 @@ _git_rebase ()
>  --*)
>  __gitcomp "
>  --onto --merge --strategy --interactive
> - --preserve-merges --stat --no-stat
> + --rebase-merges --preserve-merges --stat --no-stat
>  --committer-date-is-author-date --ignore-date
>  --ignore-whitespace --whitespace=
>  --autosquash --no-autosquash
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index ccd5254d1c9..7a3daf3e40c 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -970,6 +970,7 @@ git_rebase__interactive () {
>  init_revisions_and_shortrevisions
>
>  git rebase--helper --make-script ${keep_empty:+--keep-empty} \
> + ${rebase_merges:+--rebase-merges} \
>  $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
>  die "$(gettext "Could not generate todo list")"
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index fb64ee1fe42..a64460fd25a 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -17,6 +17,7 @@ q,quiet!           be quiet. implies --no-stat
> autostash          automatically stash/stash pop before and after
> fork-point         use 'merge-base --fork-point' to refine upstream
> onto=!             rebase onto given branch instead of upstream
> +r,rebase-merges!   try to rebase merges instead of skipping them
> p,preserve-merges! try to recreate merges instead of ignoring them
> s,strategy=!       use the given merge strategy
> no-ff!             cherry-pick all commits, even if unchanged
> @@ -88,6 +89,7 @@ type=
> state_dir=
> # One of {'', continue, skip, abort}, as parsed from command line
> action=
> +rebase_merges=
> preserve_merges=
> autosquash=
> keep_empty=
> @@ -270,6 +272,10 @@ do
>  --allow-empty-message)
>  allow_empty_message=--allow-empty-message
>  ;;
> + --rebase-merges)
> + rebase_merges=t
> + test -z "$interactive_rebase" && interactive_rebase=implied
> + ;;
>  --preserve-merges)
>  preserve_merges=t
>  test -z "$interactive_rebase" && interactive_rebase=implied
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> new file mode 100755
> index 00000000000..5f0febb9970
> --- /dev/null
> +++ b/t/t3430-rebase-merges.sh
> @@ -0,0 +1,179 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2018 Johannes E. Schindelin
> +#
> +
> +test_description='git rebase -i --rebase-merges
> +
> +This test runs git rebase "interactively", retaining the branch structure 
> by
> +recreating merge commits.
> +
> +Initial setup:
> +
> +    -- B --                   (first)
> +   /       \
> + A - C - D - E - H            (master)
> +       \       /
> +         F - G                (second)
> +'
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
> +
> +test_cmp_graph () {
> + cat >expect &&
> + git log --graph --boundary --format=%s "$@" >output &&
> + sed "s/ *$//" <output >output.trimmed &&
> + test_cmp expect output.trimmed
> +}
> +
> +test_expect_success 'setup' '
> + write_script replace-editor.sh <<-\EOF &&
> + mv "$1" "$(git rev-parse --git-path ORIGINAL-TODO)"
> + cp script-from-scratch "$1"
> + EOF
> +
> + test_commit A &&
> + git checkout -b first &&
> + test_commit B &&
> + git checkout master &&
> + test_commit C &&
> + test_commit D &&
> + git merge --no-commit B &&
> + test_tick &&
> + git commit -m E &&
> + git tag -m E E &&
> + git checkout -b second C &&
> + test_commit F &&
> + test_commit G &&
> + git checkout master &&
> + git merge --no-commit G &&
> + test_tick &&
> + git commit -m H &&
> + git tag -m H H
> +'
> +
> +test_expect_success 'create completely different structure' '
> + cat >script-from-scratch <<-\EOF &&
> + label onto
> +
> + # onebranch
> + pick G
> + pick D
> + label onebranch
> +
> + # second
> + reset onto
> + pick B
> + label second
> +
> + reset onto
> + merge -C H second
> + merge onebranch # Merge the topic branch '\''onebranch'\''
> + EOF
> + test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
> + test_tick &&
> + git rebase -i -r A &&
> + test_cmp_graph <<-\EOF
> + *   Merge the topic branch '\''onebranch'\''
> + |\
> + | * D
> + | * G
> + * |   H
> + |\ \
> + | |/
> + |/|
> + | * B
> + |/
> + * A
> + EOF
> +'
> +
> +test_expect_success 'generate correct todo list' '
> + cat >expect <<-\EOF &&
> + label onto
> +
> + reset onto
> + pick d9df450 B
> + label E
> +
> + reset onto
> + pick 5dee784 C
> + label branch-point
> + pick ca2c861 F
> + pick 088b00a G
> + label H
> +
> + reset branch-point # C
> + pick 12bd07b D
> + merge -C 2051b56 E # E
> + merge -C 233d48a H # H
> +
> + EOF
> +
> + grep -v "^#" <.git/ORIGINAL-TODO >output &&
> + test_cmp expect output
> +'
> +
> +test_expect_success '`reset` refuses to overwrite untracked files' '
> + git checkout -b refuse-to-reset &&
> + test_commit dont-overwrite-untracked &&
> + git checkout @{-1} &&
> + : >dont-overwrite-untracked.t &&
> + echo "reset refs/tags/dont-overwrite-untracked" >script-from-scratch &&
> + test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
> + test_must_fail git rebase -r HEAD &&
> + git rebase --abort
> +'
> +
> +test_expect_success 'failed `merge` writes patch (may be rescheduled, 
> too)' '
> + test_when_finished "test_might_fail git rebase --abort" &&
> + git checkout -b conflicting-merge A &&
> +
> + : fail because of conflicting untracked file &&
> + >G.t &&
> + echo "merge -C H G" >script-from-scratch &&
> + test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
> + test_tick &&
> + test_must_fail git rebase -ir HEAD &&
> + grep "^merge -C .* G$" .git/rebase-merge/done &&
> + grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
> + test_path_is_file .git/rebase-merge/patch &&
> +
> + : fail because of merge conflict &&
> + rm G.t .git/rebase-merge/patch &&
> + git reset --hard &&
> + test_commit conflicting-G G.t not-G conflicting-G &&
> + test_must_fail git rebase --continue &&
> + ! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
> + test_path_is_file .git/rebase-merge/patch
> +'
> +
> +test_expect_success 'with a branch tip that was cherry-picked already' '
> + git checkout -b already-upstream master &&
> + base="$(git rev-parse --verify HEAD)" &&
> +
> + test_commit A1 &&
> + test_commit A2 &&
> + git reset --hard $base &&
> + test_commit B1 &&
> + test_tick &&
> + git merge -m "Merge branch A" A2 &&
> +
> + git checkout -b upstream-with-a2 $base &&
> + test_tick &&
> + git cherry-pick A2 &&
> +
> + git checkout already-upstream &&
> + test_tick &&
> + git rebase -i -r upstream-with-a2 &&
> + test_cmp_graph upstream-with-a2.. <<-\EOF
> + *   Merge branch A
> + |\
> + | * A1
> + * | B1
> + |/
> + o A2
> + EOF
> +'
> +
> +test_done
> -- 
> 2.17.0.windows.1.15.gaa56ade3205
>
>
>
[1]https://public-inbox.org/git/2FA1998250474E76A386B82AD635E56A@PhilipOakley/ 


^ permalink raw reply

* [PATCH 07/24] hibernate: Disable when the kernel is locked down
From: Andy Lutomirski @ 2018-04-22 14:34 UTC (permalink / raw)
  To: linux-security-module
In-Reply-To: <27926.1524148733@warthog.procyon.org.uk>

On Thu, Apr 19, 2018 at 7:38 AM, David Howells <dhowells@redhat.com> wrote:
> Pavel Machek <pavel@ucw.cz> wrote:
>
>> > There is currently no way to verify the resume image when returning
>> > from hibernate.  This might compromise the signed modules trust model,
>> > so until we can work with signed hibernate images we disable it when the
>> > kernel is locked down.
>>
>> I'd rather see hibernation fixed than disabled like this.
>
> The problem is that you have to store the hibernated kernel image encrypted,
> but you can't store the decryption key on disk unencrypted or you've just
> wasted the effort.
>
> So the firmware has to unlock the image, asking the user for a password to
> unlock the key.

Why firmware?

Either the boot kernel could figure out how to ask for a password (or
unseal using the TPM) or we could defer this to userspace.  The latter
should already work using kexec-jump, no?
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 07/24] hibernate: Disable when the kernel is locked down
From: Andy Lutomirski @ 2018-04-22 14:34 UTC (permalink / raw)
  To: David Howells
  Cc: Pavel Machek, Linus Torvalds, linux-man, Linux API, James Morris,
	LKML, LSM List
In-Reply-To: <27926.1524148733@warthog.procyon.org.uk>

On Thu, Apr 19, 2018 at 7:38 AM, David Howells <dhowells@redhat.com> wrote:
> Pavel Machek <pavel@ucw.cz> wrote:
>
>> > There is currently no way to verify the resume image when returning
>> > from hibernate.  This might compromise the signed modules trust model,
>> > so until we can work with signed hibernate images we disable it when the
>> > kernel is locked down.
>>
>> I'd rather see hibernation fixed than disabled like this.
>
> The problem is that you have to store the hibernated kernel image encrypted,
> but you can't store the decryption key on disk unencrypted or you've just
> wasted the effort.
>
> So the firmware has to unlock the image, asking the user for a password to
> unlock the key.

Why firmware?

Either the boot kernel could figure out how to ask for a password (or
unseal using the TPM) or we could defer this to userspace.  The latter
should already work using kexec-jump, no?

^ permalink raw reply

* Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API
From: David Julian Veenstra @ 2018-04-22 14:32 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: devel, devicetree, lars, Michael.Hennerich, linux-iio, robh+dt,
	pmeerw, knaack.h, daniel.baluta
In-Reply-To: <20180421175809.01e56a52@archlinux>


On 21, April 2018 18:58, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:44 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The legacy, integer based gpio API is replaced with the descriptor
>> based API.
>> 
>> For compatibility, it is first tried to use the platform data to
>> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
>> gpio function.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> I would suggest that we simply force any out of tree users of the
> platform data to update the platform data.  Drop the the old
> versions entirely..
>
> We can be mean to out of tree board files so let us make our own
> lives easy.

Alright, I'll remove usage of platform data for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 11ed9c7332e6..29a9bb666e7b 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>>  #include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>>  #include <linux/spi/spi.h>
>> @@ -44,8 +45,8 @@
>>  struct ad2s1200_state {
>>  	struct mutex lock;
>>  	struct spi_device *sdev;
>> -	int sample;
>> -	int rdvel;
>> +	struct gpio_desc *sample;
>> +	struct gpio_desc *rdvel;
>>  	u8 rx[2] ____cacheline_aligned;
>>  };
>>  
>> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>> -	gpio_set_value(st->sample, 0);
>> +	gpiod_set_value(st->sample, 0);
>>  
>>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>>  	udelay(1);
>> -	gpio_set_value(st->sample, 1);
>> -	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +	gpiod_set_value(st->sample, 1);
>> +	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>>  
>>  	ret = spi_read(st->sdev, st->rx, 2);
>>  	if (ret < 0) {
>> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  
>>  	dev = &spi->dev;
>>  
>> -	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>> -					    DRV_NAME);
>> -		if (ret) {
>> -			dev_err(dev, "request gpio pin %d failed\n",
>> -				pins[pn]);
>> -			return ret;
>> +	if (pins) {
>> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
>> +			ret = devm_gpio_request_one(dev, pins[pn],
>> +						    GPIOF_DIR_OUT,
>> +						    DRV_NAME);
>> +			if (ret) {
>> +				dev_err(dev,
>> +					"Failed to claim gpio %d\n: err=%d",
>> +					pins[pn],
>> +					ret);
>> +				return ret;
>> +			}
>>  		}
>>  	}
>>  
>> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st = iio_priv(indio_dev);
>>  	mutex_init(&st->lock);
>>  	st->sdev = spi;
>> -	st->sample = pins[0];
>> -	st->rdvel = pins[1];
>> +
>> +	if (pins) {
>> +		st->sample = gpio_to_desc(pins[0]);
>> +		st->rdvel = gpio_to_desc(pins[1]);
>> +	} else {
>> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->sample)) {
>> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
>> +				PTR_ERR(st->sample));
>> +			return PTR_ERR(st->sample);
>> +		}
>> +
>> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->rdvel)) {
>> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
>> +				PTR_ERR(st->rdvel));
>> +			return PTR_ERR(st->rdvel);
>> +		}
>> +	}
>>  
>>  	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;

^ permalink raw reply

* Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API
From: David Julian Veenstra @ 2018-04-22 14:32 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, pmeerw, robh+dt, Michael.Hennerich, knaack.h, daniel.baluta,
	linux-iio, devel, devicetree
In-Reply-To: <20180421175809.01e56a52@archlinux>


On 21, April 2018 18:58, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:44 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The legacy, integer based gpio API is replaced with the descriptor
>> based API.
>> 
>> For compatibility, it is first tried to use the platform data to
>> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
>> gpio function.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> I would suggest that we simply force any out of tree users of the
> platform data to update the platform data.  Drop the the old
> versions entirely..
>
> We can be mean to out of tree board files so let us make our own
> lives easy.

Alright, I'll remove usage of platform data for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 11ed9c7332e6..29a9bb666e7b 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>>  #include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>>  #include <linux/spi/spi.h>
>> @@ -44,8 +45,8 @@
>>  struct ad2s1200_state {
>>  	struct mutex lock;
>>  	struct spi_device *sdev;
>> -	int sample;
>> -	int rdvel;
>> +	struct gpio_desc *sample;
>> +	struct gpio_desc *rdvel;
>>  	u8 rx[2] ____cacheline_aligned;
>>  };
>>  
>> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>> -	gpio_set_value(st->sample, 0);
>> +	gpiod_set_value(st->sample, 0);
>>  
>>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>>  	udelay(1);
>> -	gpio_set_value(st->sample, 1);
>> -	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +	gpiod_set_value(st->sample, 1);
>> +	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>>  
>>  	ret = spi_read(st->sdev, st->rx, 2);
>>  	if (ret < 0) {
>> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  
>>  	dev = &spi->dev;
>>  
>> -	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>> -					    DRV_NAME);
>> -		if (ret) {
>> -			dev_err(dev, "request gpio pin %d failed\n",
>> -				pins[pn]);
>> -			return ret;
>> +	if (pins) {
>> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
>> +			ret = devm_gpio_request_one(dev, pins[pn],
>> +						    GPIOF_DIR_OUT,
>> +						    DRV_NAME);
>> +			if (ret) {
>> +				dev_err(dev,
>> +					"Failed to claim gpio %d\n: err=%d",
>> +					pins[pn],
>> +					ret);
>> +				return ret;
>> +			}
>>  		}
>>  	}
>>  
>> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st = iio_priv(indio_dev);
>>  	mutex_init(&st->lock);
>>  	st->sdev = spi;
>> -	st->sample = pins[0];
>> -	st->rdvel = pins[1];
>> +
>> +	if (pins) {
>> +		st->sample = gpio_to_desc(pins[0]);
>> +		st->rdvel = gpio_to_desc(pins[1]);
>> +	} else {
>> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->sample)) {
>> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
>> +				PTR_ERR(st->sample));
>> +			return PTR_ERR(st->sample);
>> +		}
>> +
>> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->rdvel)) {
>> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
>> +				PTR_ERR(st->rdvel));
>> +			return PTR_ERR(st->rdvel);
>> +		}
>> +	}
>>  
>>  	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;

^ permalink raw reply

* Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup
From: David Julian Veenstra @ 2018-04-22 14:31 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: devel, devicetree, lars, Michael.Hennerich, linux-iio, robh+dt,
	pmeerw, knaack.h, daniel.baluta
In-Reply-To: <20180421175521.5592e47a@archlinux>


On 21, April 2018 18:55, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:32 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The manual states that the data is contained in the upper 12 bits
>> of the 16 bits read by spi. The code that extracts these 12 bits
>> is correct for both be and le machines, but this is not clear
>> from a first glance.
>> 
>> To improve readability the relevant expressions are replaced
>> with equivalent expressions that use be16_to_cpup.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 0a5fc9917e32..11ed9c7332e6 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  {
>>  	struct ad2s1200_state *st = iio_priv(indio_dev);
>>  	int ret = 0;
>> -	s16 vel;
>> +	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>>  	gpio_set_value(st->sample, 0);
>> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  	}
>>  
>> +	vel = be16_to_cpup((__be16 *)st->rx);
> Well it isn't vel (velocity) in one case so now the name is misleading.
>
> Also that type cast suggests a fairly obvious cleanup associated with this one.
> Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?
>
> Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

Agreed on naming of vel, and type of st-rx. I'll change this for v3.

Best regards,
David Veenstra

>
>>  	switch (chan->type) {
>>  	case IIO_ANGL:
>> -		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> +		*val = vel >> 4;
>>  		break;
>>  	case IIO_ANGL_VEL:
>> -		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> -		vel = sign_extend32(vel, 11);
>> -		*val = vel;
>> +		*val = sign_extend32((s16)vel >> 4, 11);
> If you were to use an intermediate that was s16 then the sign extend would
> be automatic.  Perhaps it is clear to do it like this though.. 
> Up to you.
>
>>  		break;
>>  	default:
>>  		mutex_unlock(&st->lock);

^ permalink raw reply

* Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup
From: David Julian Veenstra @ 2018-04-22 14:31 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, pmeerw, robh+dt, Michael.Hennerich, knaack.h, daniel.baluta,
	linux-iio, devel, devicetree
In-Reply-To: <20180421175521.5592e47a@archlinux>


On 21, April 2018 18:55, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:32 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The manual states that the data is contained in the upper 12 bits
>> of the 16 bits read by spi. The code that extracts these 12 bits
>> is correct for both be and le machines, but this is not clear
>> from a first glance.
>> 
>> To improve readability the relevant expressions are replaced
>> with equivalent expressions that use be16_to_cpup.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 0a5fc9917e32..11ed9c7332e6 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  {
>>  	struct ad2s1200_state *st = iio_priv(indio_dev);
>>  	int ret = 0;
>> -	s16 vel;
>> +	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>>  	gpio_set_value(st->sample, 0);
>> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  	}
>>  
>> +	vel = be16_to_cpup((__be16 *)st->rx);
> Well it isn't vel (velocity) in one case so now the name is misleading.
>
> Also that type cast suggests a fairly obvious cleanup associated with this one.
> Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?
>
> Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

Agreed on naming of vel, and type of st-rx. I'll change this for v3.

Best regards,
David Veenstra

>
>>  	switch (chan->type) {
>>  	case IIO_ANGL:
>> -		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> +		*val = vel >> 4;
>>  		break;
>>  	case IIO_ANGL_VEL:
>> -		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> -		vel = sign_extend32(vel, 11);
>> -		*val = vel;
>> +		*val = sign_extend32((s16)vel >> 4, 11);
> If you were to use an intermediate that was s16 then the sign extend would
> be automatic.  Perhaps it is clear to do it like this though.. 
> Up to you.
>
>>  		break;
>>  	default:
>>  		mutex_unlock(&st->lock);

^ permalink raw reply

* Re: RFC: How should we handle un-deleted remote branches?
From: Andreas Heiduk @ 2018-04-22 14:30 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List
In-Reply-To: <87d0yrebhw.fsf@evledraar.gmail.com>

Am 22.04.2018 um 13:17 schrieb Ævar Arnfjörð Bjarmason:
> 
> On Sun, Apr 22 2018, Andreas Heiduk wrote:
> 
>> Am 20.04.2018 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
>>> But this is a possible work-around:
>>>
>>>     git init /tmp/empty.git
>>>     git remote add avar file:///tmp/empty.git
>>>     git remote prune avar
>>>     git remote remove avar
>>
>> This won't do it also?
>>
>> 	git remote prune origin
> 
> Yes, in this particular case, but that's just emergent behavior in how
> we handle refspec prunign, and the fact that it "works" is arguably a
> bug in "prune". i.e. this:

Its not emergent because "origin" is the other remote responsible for
that ref and cleaning stuff "belonging" to the remote is the job
description (I'm arguing from a user's perspective, not as a git-developer).

> 
>     (
>         rm -rf /tmp/git &&
>         git clone --bare --mirror git@github.com:git/git.git /tmp/git &&
>         cd /tmp/git &&
>         git remote add avar git@github.com:avar/git.git &&
>         git remote add peff git@github.com:peff/git.git &&
>         git fetch --all &&
>         git remote remove avar &&
>         git remote prune origin
>     )
> 
> Will delete all the avar/* and peff/* branches, even though I still have
> a "peff" remote.

Exactly my point: When you are in a the bad situation of "shared
responsibility", then there is no easy and always correct way out,
because there are uncountable possible situations.

To give another, slightly modified example expanding the problem space:

(
    rm -rf /tmp/git &&
    git clone --bare --mirror https://github.com/git/git.git /tmp/git &&
    cd /tmp/git &&
    git remote add github https://github.com/avar/git.git &&
    git fetch github &&
    git fetch origin &&
    # note new fetches for "github/master" using with "(forced update)"
)

For ... reasons the first repo publishes some references like

	github/maint
	github/master
	github/pu

So when this repo is mirrored AND another, suitably named remote is
added then there will be also namespace conflicts. You can call

    git fetch github
    git fetch origin

in a loop and most likely each fetch will update the same refs, always
toggling between two states.

So: not only "remote remove" and "remote prune" are at stake but every
command handling remote references.

How should "git remote remove github" work in both situations? Remove
the refs/remotes/github/master & co? remove them only if the last fetch
was for "github" but not when the last update was for "origin"? Should
"git fetch" use the same logic?

So it seems better to me to avoid that bad situation altogether. Don't
allow overlapping/conflicting refspecs when adding a new remote. Using
*your* last examples both

>         git remote add avar git@github.com:avar/git.git &&
>         git remote add peff git@github.com:peff/git.git &&

should have failed and hence the "prune" problems won't exist. Same for
my example.

>> Possible 5):
>>
>> 	Don't fix "git remote remove" but "git remote add" to complain that its
>> ref-namespace is already occupied by some other remote. Add "--force"
>> for the experts.
> 
> Indeed, that's another bug here, i.e. in the above example:
> 
>     git remote remove peff && # won't delete peff/ branches
>     git remote add peff git@github.com:peff/git.git
> 
> Will happily add the "peff" remote again, even though as you point out
> it could be an entirely different remote.

Ummm. That was not my point. My is: "git clone --mirror" uses a refspec

	fetch = +refs/*:refs/*

and hence "occupies" the complete "refs/*" namespace. So adding another
remote (for the first time or for the second time is as irrelevant as
the url) will use

	fetch = +refs/heads/*:refs/remotes/peff/*

and now the "refs/remotes/peff/*" part is in conflict with "refs/*" from
above. The conflict exists not only for "prune" or "remove" but also for
"fetch", "rename" (didn't check) .

This kind of conflict should not be allowed right from the start - when
the first "git remote add peff..." is executed. Then prune, remove AND
fetch would be OK.



^ permalink raw reply

* Re: [PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value
From: David Julian Veenstra @ 2018-04-22 14:29 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, pmeerw, robh+dt, Michael.Hennerich, knaack.h, daniel.baluta,
	linux-iio, devel, devicetree
In-Reply-To: <20180421175051.05f7e1b1@archlinux>


On 21, April 2018 18:50, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:19 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
>> used in ad2s1200_probe.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> No significant gain in readability.   Perhaps even a slight lost I'm
> going to say no to this one.

We discussed this in [1]. But after removal of platform data it will
be even less worth it. I'll remove this patch in v3.

Best regards,
David Veenstra

[1] https://marc.info/?l=linux-iio&m=152190029808229&w=2

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index f07aab7e7a35..0a5fc9917e32 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	unsigned short *pins = spi->dev.platform_data;
>>  	struct ad2s1200_state *st;
>>  	struct iio_dev *indio_dev;
>> +	struct device *dev;
>>  	int pn, ret = 0;
>>  
>> +	dev = &spi->dev;
>> +
>>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
>> +		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>>  					    DRV_NAME);
>>  		if (ret) {
>> -			dev_err(&spi->dev, "request gpio pin %d failed\n",
>> +			dev_err(dev, "request gpio pin %d failed\n",
>>  				pins[pn]);
>>  			return ret;
>>  		}
>>  	}
>>  
>> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>>  	if (!indio_dev)
>>  		return -ENOMEM;
>>  
>> @@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st->sample = pins[0];
>>  	st->rdvel = pins[1];
>>  
>> -	indio_dev->dev.parent = &spi->dev;
>> +	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;
>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>  	indio_dev->channels = ad2s1200_channels;
>>  	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>>  	indio_dev->name = spi_get_device_id(spi)->name;
>>  
>> -	ret = devm_iio_device_register(&spi->dev, indio_dev);
>> +	ret = devm_iio_device_register(dev, indio_dev);
>>  	if (ret)
>>  		return ret;
>>  


^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.