* [PATCH v2 0/2] iio: pressure: do not rely on structure field ordering @ 2017-02-01 20:40 Peter Rosin 2017-02-01 20:40 ` [PATCH v2 1/2] iio: pressure: mpl3115: " Peter Rosin 2017-02-01 20:40 ` [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering Peter Rosin 0 siblings, 2 replies; 9+ messages in thread From: Peter Rosin @ 2017-02-01 20:40 UTC (permalink / raw) To: linux-kernel Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Alison Schofield, Ken Lin, linux-iio Hi! v1 -> v2 changes: - improved commit messages - added tested-by tag These two patches are needed to fix some fallout from adding the available attribute. Jonathan, it was a bitch to rebase with that line starting with a #. Please make sure that it stays where it should be in case you decide to fixup anything... Cheers, peda Peter Rosin (2): iio: pressure: mpl3115: do not rely on structure field ordering iio: pressure: mpl115: do not rely on structure field ordering drivers/iio/pressure/mpl115.c | 1 + drivers/iio/pressure/mpl3115.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] iio: pressure: mpl3115: do not rely on structure field ordering 2017-02-01 20:40 [PATCH v2 0/2] iio: pressure: do not rely on structure field ordering Peter Rosin @ 2017-02-01 20:40 ` Peter Rosin 2017-02-05 9:37 ` Jonathan Cameron 2017-02-01 20:40 ` [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering Peter Rosin 1 sibling, 1 reply; 9+ messages in thread From: Peter Rosin @ 2017-02-01 20:40 UTC (permalink / raw) To: linux-kernel Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Alison Schofield, Ken Lin, linux-iio Fixes a regression triggered by a change in the layout of struct iio_chan_spec, but the real bug is in the driver which assumed a specific structure layout in the first place. Hint: the two bits were not OR:ed together as implied by the indentation prior to this patch, there was a comma between them, which accidentally moved the ..._SCALE bit to the next structure field. That field was .info_mask_shared_by_type before the _available attributes was added by commit 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes") and .info_mask_separate_available afterwards, and the regression happened. info_mask_shared_by_type is actually a better choice than the originally intended info_mask_separate for the ..._SCALE bit since a constant is returned from mpl3115_read_raw for the scale. Using info_mask_shared_by_type also preserves the behavior from before the regression and is therefore less likely to cause other interesting side effects. The above mentioned regression causes an unintended sysfs attibute to show up that is not backed by code, in turn causing the following NULL pointer defererence to happen on access. # cat /sys/bus/iio/devices/iio\:device1/in_pressure_scale_available Segmentation fault Unable to handle kernel NULL pointer dereference at virtual address 00000000 pgd = ecc3c000 [00000000] *pgd=87f91831 Internal error: Oops: 80000007 [#1] SMP ARM Modules linked in: CPU: 1 PID: 1051 Comm: cat Not tainted 4.10.0-rc5-00009-gffd8858-dirty #3 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) task: ed54ec00 task.stack: ee2bc000 PC is at 0x0 LR is at iio_read_channel_info_avail+0x40/0x280 pc : [<00000000>] lr : [<c06fbc1c>] psr: a0070013 sp : ee2bdda8 ip : 00000000 fp : ee2bddf4 r10: c0a53c74 r9 : ed79f000 r8 : ee8d1018 r7 : 00001000 r6 : 00000fff r5 : ee8b9a00 r4 : ed79f000 r3 : ee2bddc4 r2 : ee2bddbc r1 : c0a86dcc r0 : ee8d1000 Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 3cc3c04a DAC: 00000051 Process cat (pid: 1051, stack limit = 0xee2bc210) Stack: (0xee2bdda8 to 0xee2be000) dda0: ee2bddc0 00000002 c016d720 c016d394 ed54ec00 00000000 ddc0: 60070013 ed413780 00000001 edffd480 ee8b9a00 00000fff 00001000 ee8d1018 dde0: ed79f000 c0a53c74 ee2bde0c ee2bddf8 c0513c58 c06fbbe8 edffd480 edffd540 de00: ee2bde3c ee2bde10 c0293474 c0513c40 c02933e4 ee2bde60 00000001 ed413780 de20: 00000001 ed413780 00000000 edffd480 ee2bde4c ee2bde40 c0291d00 c02933f0 de40: ee2bde9c ee2bde50 c024679c c0291ce0 edffd4b0 b6e37000 00020000 ee2bdf78 de60: 00000000 00000000 ed54ec00 ed013200 00000817 c0a111fc edffd540 ed413780 de80: b6e37000 00020000 00020000 ee2bdf78 ee2bded4 ee2bdea0 c0292890 c0246604 dea0: c0117940 c016ba50 00000025 c0a111fc b6e37000 ed413780 ee2bdf78 00020000 dec0: ee2bc000 b6e37000 ee2bdf44 ee2bded8 c021d158 c0292770 c0117764 b6e36004 dee0: c0f0d7c4 ee2bdfb0 b6f89228 00021008 ee2bdfac ee2bdf00 c0101374 c0117770 df00: 00000000 00000000 ee2bc000 00000000 ee2bdf34 ee2bdf20 c016ba04 c0171080 df20: 00000000 00020000 ed413780 b6e37000 00000000 ee2bdf78 ee2bdf74 ee2bdf48 df40: c021e7a0 c021d130 c023e300 c023e280 ee2bdf74 00000000 00000000 ed413780 df60: ed413780 00020000 ee2bdfa4 ee2bdf78 c021e870 c021e71c 00000000 00000000 df80: 00020000 00020000 b6e37000 00000003 c0108084 00000000 00000000 ee2bdfa8 dfa0: c0107ee0 c021e838 00020000 00020000 00000003 b6e37000 00020000 0001a2b4 dfc0: 00020000 00020000 b6e37000 00000003 7fffe000 00000000 00000000 00020000 dfe0: 00000000 be98eb4c 0000c740 b6f1985c 60070010 00000003 00000000 00000000 Backtrace: [<c06fbbdc>] (iio_read_channel_info_avail) from [<c0513c58>] (dev_attr_show+0x24/0x50) r10:c0a53c74 r9:ed79f000 r8:ee8d1018 r7:00001000 r6:00000fff r5:ee8b9a00 r4:edffd480 [<c0513c34>] (dev_attr_show) from [<c0293474>] (sysfs_kf_seq_show+0x90/0x110) r5:edffd540 r4:edffd480 [<c02933e4>] (sysfs_kf_seq_show) from [<c0291d00>] (kernfs_seq_show+0x2c/0x30) r10:edffd480 r9:00000000 r8:ed413780 r7:00000001 r6:ed413780 r5:00000001 r4:ee2bde60 r3:c02933e4 [<c0291cd4>] (kernfs_seq_show) from [<c024679c>] (seq_read+0x1a4/0x4e0) [<c02465f8>] (seq_read) from [<c0292890>] (kernfs_fop_read+0x12c/0x1cc) r10:ee2bdf78 r9:00020000 r8:00020000 r7:b6e37000 r6:ed413780 r5:edffd540 r4:c0a111fc [<c0292764>] (kernfs_fop_read) from [<c021d158>] (__vfs_read+0x34/0x118) r10:b6e37000 r9:ee2bc000 r8:00020000 r7:ee2bdf78 r6:ed413780 r5:b6e37000 r4:c0a111fc [<c021d124>] (__vfs_read) from [<c021e7a0>] (vfs_read+0x90/0x11c) r8:ee2bdf78 r7:00000000 r6:b6e37000 r5:ed413780 r4:00020000 [<c021e710>] (vfs_read) from [<c021e870>] (SyS_read+0x44/0x90) r8:00020000 r7:ed413780 r6:ed413780 r5:00000000 r4:00000000 [<c021e82c>] (SyS_read) from [<c0107ee0>] (ret_fast_syscall+0x0/0x1c) r10:00000000 r8:c0108084 r7:00000003 r6:b6e37000 r5:00020000 r4:00020000 Code: bad PC value ---[ end trace 9c4938ccd0389004 ]--- Fixes: cc26ad455f57 ("iio: Add Freescale MPL3115A2 pressure / temperature sensor driver") Fixes: 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes") Reported-by: Ken Lin <ken.lin@advantech.com> Tested-by: Ken Lin <ken.lin@advantech.com> Signed-off-by: Peter Rosin <peda@axentia.se> --- drivers/iio/pressure/mpl3115.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c index cc3f84139157..525644a7442d 100644 --- a/drivers/iio/pressure/mpl3115.c +++ b/drivers/iio/pressure/mpl3115.c @@ -190,7 +190,7 @@ static const struct iio_chan_spec mpl3115_channels[] = { { .type = IIO_PRESSURE, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), - BIT(IIO_CHAN_INFO_SCALE), + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), .scan_index = 0, .scan_type = { .sign = 'u', @@ -203,7 +203,7 @@ static const struct iio_chan_spec mpl3115_channels[] = { { .type = IIO_TEMP, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), - BIT(IIO_CHAN_INFO_SCALE), + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), .scan_index = 1, .scan_type = { .sign = 's', -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] iio: pressure: mpl3115: do not rely on structure field ordering 2017-02-01 20:40 ` [PATCH v2 1/2] iio: pressure: mpl3115: " Peter Rosin @ 2017-02-05 9:37 ` Jonathan Cameron 2017-02-10 22:35 ` [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes Peter Rosin 0 siblings, 1 reply; 9+ messages in thread From: Jonathan Cameron @ 2017-02-05 9:37 UTC (permalink / raw) To: Peter Rosin, linux-kernel Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Alison Schofield, Ken Lin, linux-iio On 01/02/17 20:40, Peter Rosin wrote: > Fixes a regression triggered by a change in the layout of > struct iio_chan_spec, but the real bug is in the driver which assumed > a specific structure layout in the first place. Hint: the two bits were > not OR:ed together as implied by the indentation prior to this patch, > there was a comma between them, which accidentally moved the ..._SCALE > bit to the next structure field. That field was .info_mask_shared_by_type > before the _available attributes was added by commit 51239600074b > ("iio:core: add a callback to allow drivers to provide _available > attributes") and .info_mask_separate_available afterwards, and the > regression happened. > > info_mask_shared_by_type is actually a better choice than the originally > intended info_mask_separate for the ..._SCALE bit since a constant is > returned from mpl3115_read_raw for the scale. Using > info_mask_shared_by_type also preserves the behavior from before the > regression and is therefore less likely to cause other interesting side > effects. > > The above mentioned regression causes an unintended sysfs attibute to > show up that is not backed by code, in turn causing the following NULL > pointer defererence to happen on access. > > # cat /sys/bus/iio/devices/iio\:device1/in_pressure_scale_available > Segmentation fault > > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > pgd = ecc3c000 > [00000000] *pgd=87f91831 > Internal error: Oops: 80000007 [#1] SMP ARM > Modules linked in: > CPU: 1 PID: 1051 Comm: cat Not tainted 4.10.0-rc5-00009-gffd8858-dirty #3 > Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > task: ed54ec00 task.stack: ee2bc000 > PC is at 0x0 > LR is at iio_read_channel_info_avail+0x40/0x280 > pc : [<00000000>] lr : [<c06fbc1c>] psr: a0070013 > sp : ee2bdda8 ip : 00000000 fp : ee2bddf4 > r10: c0a53c74 r9 : ed79f000 r8 : ee8d1018 > r7 : 00001000 r6 : 00000fff r5 : ee8b9a00 r4 : ed79f000 > r3 : ee2bddc4 r2 : ee2bddbc r1 : c0a86dcc r0 : ee8d1000 > Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 3cc3c04a DAC: 00000051 > Process cat (pid: 1051, stack limit = 0xee2bc210) > Stack: (0xee2bdda8 to 0xee2be000) > dda0: ee2bddc0 00000002 c016d720 c016d394 ed54ec00 00000000 > ddc0: 60070013 ed413780 00000001 edffd480 ee8b9a00 00000fff 00001000 ee8d1018 > dde0: ed79f000 c0a53c74 ee2bde0c ee2bddf8 c0513c58 c06fbbe8 edffd480 edffd540 > de00: ee2bde3c ee2bde10 c0293474 c0513c40 c02933e4 ee2bde60 00000001 ed413780 > de20: 00000001 ed413780 00000000 edffd480 ee2bde4c ee2bde40 c0291d00 c02933f0 > de40: ee2bde9c ee2bde50 c024679c c0291ce0 edffd4b0 b6e37000 00020000 ee2bdf78 > de60: 00000000 00000000 ed54ec00 ed013200 00000817 c0a111fc edffd540 ed413780 > de80: b6e37000 00020000 00020000 ee2bdf78 ee2bded4 ee2bdea0 c0292890 c0246604 > dea0: c0117940 c016ba50 00000025 c0a111fc b6e37000 ed413780 ee2bdf78 00020000 > dec0: ee2bc000 b6e37000 ee2bdf44 ee2bded8 c021d158 c0292770 c0117764 b6e36004 > dee0: c0f0d7c4 ee2bdfb0 b6f89228 00021008 ee2bdfac ee2bdf00 c0101374 c0117770 > df00: 00000000 00000000 ee2bc000 00000000 ee2bdf34 ee2bdf20 c016ba04 c0171080 > df20: 00000000 00020000 ed413780 b6e37000 00000000 ee2bdf78 ee2bdf74 ee2bdf48 > df40: c021e7a0 c021d130 c023e300 c023e280 ee2bdf74 00000000 00000000 ed413780 > df60: ed413780 00020000 ee2bdfa4 ee2bdf78 c021e870 c021e71c 00000000 00000000 > df80: 00020000 00020000 b6e37000 00000003 c0108084 00000000 00000000 ee2bdfa8 > dfa0: c0107ee0 c021e838 00020000 00020000 00000003 b6e37000 00020000 0001a2b4 > dfc0: 00020000 00020000 b6e37000 00000003 7fffe000 00000000 00000000 00020000 > dfe0: 00000000 be98eb4c 0000c740 b6f1985c 60070010 00000003 00000000 00000000 > Backtrace: > [<c06fbbdc>] (iio_read_channel_info_avail) from [<c0513c58>] (dev_attr_show+0x24/0x50) > r10:c0a53c74 r9:ed79f000 r8:ee8d1018 r7:00001000 r6:00000fff r5:ee8b9a00 > r4:edffd480 > [<c0513c34>] (dev_attr_show) from [<c0293474>] (sysfs_kf_seq_show+0x90/0x110) > r5:edffd540 r4:edffd480 > [<c02933e4>] (sysfs_kf_seq_show) from [<c0291d00>] (kernfs_seq_show+0x2c/0x30) > r10:edffd480 r9:00000000 r8:ed413780 r7:00000001 r6:ed413780 r5:00000001 > r4:ee2bde60 r3:c02933e4 > [<c0291cd4>] (kernfs_seq_show) from [<c024679c>] (seq_read+0x1a4/0x4e0) > [<c02465f8>] (seq_read) from [<c0292890>] (kernfs_fop_read+0x12c/0x1cc) > r10:ee2bdf78 r9:00020000 r8:00020000 r7:b6e37000 r6:ed413780 r5:edffd540 > r4:c0a111fc > [<c0292764>] (kernfs_fop_read) from [<c021d158>] (__vfs_read+0x34/0x118) > r10:b6e37000 r9:ee2bc000 r8:00020000 r7:ee2bdf78 r6:ed413780 r5:b6e37000 > r4:c0a111fc > [<c021d124>] (__vfs_read) from [<c021e7a0>] (vfs_read+0x90/0x11c) > r8:ee2bdf78 r7:00000000 r6:b6e37000 r5:ed413780 r4:00020000 > [<c021e710>] (vfs_read) from [<c021e870>] (SyS_read+0x44/0x90) > r8:00020000 r7:ed413780 r6:ed413780 r5:00000000 r4:00000000 > [<c021e82c>] (SyS_read) from [<c0107ee0>] (ret_fast_syscall+0x0/0x1c) > r10:00000000 r8:c0108084 r7:00000003 r6:b6e37000 r5:00020000 r4:00020000 > Code: bad PC value > ---[ end trace 9c4938ccd0389004 ]--- > > Fixes: cc26ad455f57 ("iio: Add Freescale MPL3115A2 pressure / temperature sensor driver") > Fixes: 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes") > Reported-by: Ken Lin <ken.lin@advantech.com> > Tested-by: Ken Lin <ken.lin@advantech.com> > Signed-off-by: Peter Rosin <peda@axentia.se> Applied to the fixes-togreg branch of iio.git. I'll probably get a pull request out for these later today as would be nice to avoid the regression reaching a release. Jonathan > --- > drivers/iio/pressure/mpl3115.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c > index cc3f84139157..525644a7442d 100644 > --- a/drivers/iio/pressure/mpl3115.c > +++ b/drivers/iio/pressure/mpl3115.c > @@ -190,7 +190,7 @@ static const struct iio_chan_spec mpl3115_channels[] = { > { > .type = IIO_PRESSURE, > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - BIT(IIO_CHAN_INFO_SCALE), > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .scan_index = 0, > .scan_type = { > .sign = 'u', > @@ -203,7 +203,7 @@ static const struct iio_chan_spec mpl3115_channels[] = { > { > .type = IIO_TEMP, > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - BIT(IIO_CHAN_INFO_SCALE), > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .scan_index = 1, > .scan_type = { > .sign = 's', > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes. 2017-02-05 9:37 ` Jonathan Cameron @ 2017-02-10 22:35 ` Peter Rosin 2017-02-11 7:17 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ messages in thread From: Peter Rosin @ 2017-02-10 22:35 UTC (permalink / raw) To: Jonathan Cameron, linux-kernel, Greg Kroah-Hartman; +Cc: Ken Lin, linux-iio > On Sun, Feb 05, 2017 at 10:35:02AM +0000, Jonathan Cameron wrote: >> The following changes since commit 5c113b5e0082e90d2e1c7b12e96a7b8cf0623e27: >> >> iio: dht11: Use usleep_range instead of msleep for start signal (2017-01-22 13:35:40 +0000) >> >> are available in the git repository at: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-fixes-for-4.10c > > It's a bit late for 4.10 for me, can I just pull this into my -next > branch and will they get to 4.10.1 properly? Meaning, do that have cc: > stable markings on them? Or do you want to fix that up and resend this > request? Hi Greg, You should ask Ken Lin who has the HW and who is apparently affected. I think it's bad that you are willing to have a known regression hit v4.10 when all was fine in v4.9. Or perhaps you didn't realize that the regression was from this cycle? The fixes are obvious. I don't understand your hesitation. Cheers, peda ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes. 2017-02-10 22:35 ` [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes Peter Rosin @ 2017-02-11 7:17 ` Greg Kroah-Hartman 2017-02-11 9:16 ` Jonathan Cameron 0 siblings, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2017-02-11 7:17 UTC (permalink / raw) To: Peter Rosin; +Cc: Jonathan Cameron, linux-kernel, Ken Lin, linux-iio On Fri, Feb 10, 2017 at 11:35:35PM +0100, Peter Rosin wrote: > > On Sun, Feb 05, 2017 at 10:35:02AM +0000, Jonathan Cameron wrote: > >> The following changes since commit 5c113b5e0082e90d2e1c7b12e96a7b8cf0623e27: > >> > >> iio: dht11: Use usleep_range instead of msleep for start signal (2017-01-22 13:35:40 +0000) > >> > >> are available in the git repository at: > >> > >> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-fixes-for-4.10c > > > > It's a bit late for 4.10 for me, can I just pull this into my -next > > branch and will they get to 4.10.1 properly? Meaning, do that have cc: > > stable markings on them? Or do you want to fix that up and resend this > > request? > > Hi Greg, > > You should ask Ken Lin who has the HW and who is apparently affected. > I think it's bad that you are willing to have a known regression hit > v4.10 when all was fine in v4.9. Or perhaps you didn't realize that > the regression was from this cycle? > > The fixes are obvious. I don't understand your hesitation. My "hesitation" is that I'm about to get on a plane for a day or so and don't have the time to get this to Linus before 4.10-final is out this Sunday. Getting it in a week later should be ok, we all make mistakes, as long as we fix them it's all good, and for 4.10.1 should be ok. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes. 2017-02-11 7:17 ` Greg Kroah-Hartman @ 2017-02-11 9:16 ` Jonathan Cameron 2017-02-14 14:29 ` Peter Rosin 0 siblings, 1 reply; 9+ messages in thread From: Jonathan Cameron @ 2017-02-11 9:16 UTC (permalink / raw) To: Greg Kroah-Hartman, Peter Rosin; +Cc: linux-kernel, Ken Lin, linux-iio On 11/02/17 07:17, Greg Kroah-Hartman wrote: > On Fri, Feb 10, 2017 at 11:35:35PM +0100, Peter Rosin wrote: >>> On Sun, Feb 05, 2017 at 10:35:02AM +0000, Jonathan Cameron wrote: >>>> The following changes since commit 5c113b5e0082e90d2e1c7b12e96a7b8cf0623e27: >>>> >>>> iio: dht11: Use usleep_range instead of msleep for start signal (2017-01-22 13:35:40 +0000) >>>> >>>> are available in the git repository at: >>>> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-fixes-for-4.10c >>> >>> It's a bit late for 4.10 for me, can I just pull this into my -next >>> branch and will they get to 4.10.1 properly? Meaning, do that have cc: >>> stable markings on them? Or do you want to fix that up and resend this >>> request? >> >> Hi Greg, >> >> You should ask Ken Lin who has the HW and who is apparently affected. >> I think it's bad that you are willing to have a known regression hit >> v4.10 when all was fine in v4.9. Or perhaps you didn't realize that >> the regression was from this cycle? >> >> The fixes are obvious. I don't understand your hesitation. > > My "hesitation" is that I'm about to get on a plane for a day or so and > don't have the time to get this to Linus before 4.10-final is out this > Sunday. Getting it in a week later should be ok, we all make mistakes, > as long as we fix them it's all good, and for 4.10.1 should be ok. Who knows, maybe Linus will delay another week anyway ;) Wasn't looking that clear when I sent these out. >From my point of view, sure 4.10.1 should be fine. I'll put stable CCs on them. Will send the pull out shortly so I don't forget about them. Pull request clearly now becomes first set of fixes for 4.11. Jonathan > > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes. 2017-02-11 9:16 ` Jonathan Cameron @ 2017-02-14 14:29 ` Peter Rosin 0 siblings, 0 replies; 9+ messages in thread From: Peter Rosin @ 2017-02-14 14:29 UTC (permalink / raw) To: Jonathan Cameron, Greg Kroah-Hartman; +Cc: linux-kernel, Ken Lin, linux-iio On 2017-02-11 10:16, Jonathan Cameron wrote: > On 11/02/17 07:17, Greg Kroah-Hartman wrote: >> On Fri, Feb 10, 2017 at 11:35:35PM +0100, Peter Rosin wrote: >>>> On Sun, Feb 05, 2017 at 10:35:02AM +0000, Jonathan Cameron wrote: >>>>> The following changes since commit 5c113b5e0082e90d2e1c7b12e96a7b8cf0623e27: >>>>> >>>>> iio: dht11: Use usleep_range instead of msleep for start signal (2017-01-22 13:35:40 +0000) >>>>> >>>>> are available in the git repository at: >>>>> >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-fixes-for-4.10c >>>> >>>> It's a bit late for 4.10 for me, can I just pull this into my -next >>>> branch and will they get to 4.10.1 properly? Meaning, do that have cc: >>>> stable markings on them? Or do you want to fix that up and resend this >>>> request? >>> >>> Hi Greg, >>> >>> You should ask Ken Lin who has the HW and who is apparently affected. >>> I think it's bad that you are willing to have a known regression hit >>> v4.10 when all was fine in v4.9. Or perhaps you didn't realize that >>> the regression was from this cycle? >>> >>> The fixes are obvious. I don't understand your hesitation. >> >> My "hesitation" is that I'm about to get on a plane for a day or so and >> don't have the time to get this to Linus before 4.10-final is out this >> Sunday. Getting it in a week later should be ok, we all make mistakes, >> as long as we fix them it's all good, and for 4.10.1 should be ok. > Who knows, maybe Linus will delay another week anyway ;) Wasn't looking > that clear when I sent these out. > > From my point of view, sure 4.10.1 should be fine. I'll put stable CCs > on them. Will send the pull out shortly so I don't forget about them. > > Pull request clearly now becomes first set of fixes for 4.11. I would like to point out that one of the commit messages was destroyed in the rebase. I.e. commit 9cf6cdba586c ("iio: pressure: mpl3115: do not rely on structure field ordering") is now missing this line: # cat /sys/bus/iio/devices/iio\:device1/in_pressure_scale_available which was present before the "Segmentation fault" line before the patch was rebased. Without the missing line, the commit message makes much less sense... Can this please be fixed? Perhaps there is even time for it to make v4.10? Cheers, peda ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering 2017-02-01 20:40 [PATCH v2 0/2] iio: pressure: do not rely on structure field ordering Peter Rosin 2017-02-01 20:40 ` [PATCH v2 1/2] iio: pressure: mpl3115: " Peter Rosin @ 2017-02-01 20:40 ` Peter Rosin 2017-02-05 9:38 ` Jonathan Cameron 1 sibling, 1 reply; 9+ messages in thread From: Peter Rosin @ 2017-02-01 20:40 UTC (permalink / raw) To: linux-kernel Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Alison Schofield, Ken Lin, linux-iio Fixes a regression triggered by a change in the layout of struct iio_chan_spec, but the real bug is in the driver which assumed a specific structure layout in the first place. Hint: the three bits were not OR:ed together as implied by the indentation prior to this patch, there was a comma between the first two, which accidentally moved the ..._SCALE and ..._OFFSET bits to the next structure field. That field was .info_mask_shared_by_type before the _available attributes was added by commit 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes") and .info_mask_separate_available afterwards, and the regression happened. info_mask_shared_by_type is actually a better choice than the originally intended info_mask_separate for the ..._SCALE and ..._OFFSET bits since a constant is returned from mpl115_read_raw for the scale/offset. Using info_mask_shared_by_type also preserves the behavior from before the regression and is therefore less likely to cause other interesting side effects. The above mentioned regression causes unintended sysfs attibutes to show up that are not backed by code, in turn causing a NULL pointer defererence to happen on access. Fixes: 3017d90e8931 ("iio: Add Freescale MPL115A2 pressure / temperature sensor driver") Fixes: 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes") Signed-off-by: Peter Rosin <peda@axentia.se> --- drivers/iio/pressure/mpl115.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/pressure/mpl115.c b/drivers/iio/pressure/mpl115.c index 73f2f0c46e62..8f2bce213248 100644 --- a/drivers/iio/pressure/mpl115.c +++ b/drivers/iio/pressure/mpl115.c @@ -137,6 +137,7 @@ static const struct iio_chan_spec mpl115_channels[] = { { .type = IIO_TEMP, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_SCALE), }, }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering 2017-02-01 20:40 ` [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering Peter Rosin @ 2017-02-05 9:38 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2017-02-05 9:38 UTC (permalink / raw) To: Peter Rosin, linux-kernel Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Alison Schofield, Ken Lin, linux-iio On 01/02/17 20:40, Peter Rosin wrote: > Fixes a regression triggered by a change in the layout of > struct iio_chan_spec, but the real bug is in the driver which assumed > a specific structure layout in the first place. Hint: the three bits were > not OR:ed together as implied by the indentation prior to this patch, > there was a comma between the first two, which accidentally moved the > ..._SCALE and ..._OFFSET bits to the next structure field. That field > was .info_mask_shared_by_type before the _available attributes was added > by commit 51239600074b ("iio:core: add a callback to allow drivers to > provide _available attributes") and .info_mask_separate_available > afterwards, and the regression happened. > > info_mask_shared_by_type is actually a better choice than the originally > intended info_mask_separate for the ..._SCALE and ..._OFFSET bits since > a constant is returned from mpl115_read_raw for the scale/offset. Using > info_mask_shared_by_type also preserves the behavior from before the > regression and is therefore less likely to cause other interesting side > effects. > > The above mentioned regression causes unintended sysfs attibutes to > show up that are not backed by code, in turn causing a NULL pointer > defererence to happen on access. > > Fixes: 3017d90e8931 ("iio: Add Freescale MPL115A2 pressure / temperature sensor driver") > Fixes: 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes") > Signed-off-by: Peter Rosin <peda@axentia.se> Applied to the fixes-togreg branch of iio.git. Thanks, Jonathan > --- > drivers/iio/pressure/mpl115.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/iio/pressure/mpl115.c b/drivers/iio/pressure/mpl115.c > index 73f2f0c46e62..8f2bce213248 100644 > --- a/drivers/iio/pressure/mpl115.c > +++ b/drivers/iio/pressure/mpl115.c > @@ -137,6 +137,7 @@ static const struct iio_chan_spec mpl115_channels[] = { > { > .type = IIO_TEMP, > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = > BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_SCALE), > }, > }; > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-02-14 14:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-01 20:40 [PATCH v2 0/2] iio: pressure: do not rely on structure field ordering Peter Rosin 2017-02-01 20:40 ` [PATCH v2 1/2] iio: pressure: mpl3115: " Peter Rosin 2017-02-05 9:37 ` Jonathan Cameron 2017-02-10 22:35 ` [PULL] IIO fixes for 4.10 set 3 - a couple of regression fixes Peter Rosin 2017-02-11 7:17 ` Greg Kroah-Hartman 2017-02-11 9:16 ` Jonathan Cameron 2017-02-14 14:29 ` Peter Rosin 2017-02-01 20:40 ` [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering Peter Rosin 2017-02-05 9:38 ` Jonathan Cameron
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.