mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
To: Peter Rosin <peda@axentia.se>
Cc: linux-kernel@vger.kernel.org, Jonathan Cameron <jic23@kernel.org>,
	Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Alison Schofield <amsfield22@gmail.com>,
	Gregor Boirie <gregor.boirie@parrot.com>,
	Sanchayan Maity <maitysanchayan@gmail.com>,
	Ken Lin <ken.lin@advantech.com>,
	linux-iio@vger.kernel.org
Subject: Re: [PATCH 1/2] iio: pressure: mpl3115: do not rely on structure field ordering
Date: Wed, 1 Feb 2017 10:31:12 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.02.1702011021580.3042@pmeerw.net> (raw)
In-Reply-To: <ef02efd6-d0da-9310-ba1b-ab3133637f6b@axentia.se>


> >> 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.

> > what do you mean by 'driver which assumed a specific structure'?
> 
> Look again, the two bits are not OR:ed together as implied by the
> indentation. There is a comma between them, which put the ..._SCALE
> bit in the next field. That next field was .info_mask_shared_by_type
> before the patch adding the available attribute that triggered the
> regression and .info_mask_separate_available after it.

wow, now that you say it :)

since I wrote these drivers I can assure you that this is just a typo, use
of the 'specific structure' is a coincident

maybe adding your explanation regarding not ORing to the patch description
would be a good idea

I was confused by the change from 
.info_mask_separate to .info_mask_shared_by_type
a fix could have just changed
-  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
			BIT(IIO_CHAN_INFO_SCALE),
+  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
			BIT(IIO_CHAN_INFO_SCALE),
as originally intended

> Cheers,
> peda
> 
> > thanks, p.
> > 
> >> 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',
> >>
> > 
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

  reply	other threads:[~2017-02-01  9:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-01  8:09 [PATCH 0/2] iio: pressure: " Peter Rosin
2017-02-01  8:09 ` [PATCH 1/2] iio: pressure: mpl3115: " Peter Rosin
2017-02-01  9:10   ` Peter Meerwald-Stadler
2017-02-01  9:13     ` Peter Rosin
2017-02-01  9:31       ` Peter Meerwald-Stadler [this message]
2017-02-01  9:43         ` Peter Rosin
2017-02-01  9:57           ` Peter Meerwald-Stadler
2017-02-01 10:18             ` Peter Rosin
     [not found]               ` <WM!e12d26e0b57c992627516bbebee1f0ac620aa97ce815547b240a5ff4318cef8602c115af08639804344b7a648bbe4b31!@dg.advantech.com>
     [not found]                 ` <03B5A3CA1724CE4EAC32B27E39292A677FC7512F94@AUSMAIL1.AUS.ADVANTECH.CORP>
2017-02-01 18:24                   ` Ken.Lin
2017-02-01 11:21   ` Jonathan Cameron
2017-02-01  8:09 ` [PATCH 2/2] iio: pressure: mpl115: " Peter Rosin
2017-02-01  8:52 ` [PATCH 0/2] iio: pressure: " Lars-Peter Clausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.02.1702011021580.3042@pmeerw.net \
    --to=pmeerw@pmeerw.net \
    --cc=amsfield22@gmail.com \
    --cc=gregor.boirie@parrot.com \
    --cc=jic23@kernel.org \
    --cc=ken.lin@advantech.com \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maitysanchayan@gmail.com \
    --cc=peda@axentia.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®