Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Anand Ashok Dumbre <ANANDASH@xilinx.com>
To:     Jonathan Cameron <jic23@kernel.org>
CC:     "knaack.h@gmx.de" <knaack.h@gmx.de>,
        "lars@metafoo.de" <lars@metafoo.de>,
        "pmeerw@pmeerw.net" <pmeerw@pmeerw.net>,
        Michal Simek <michals@xilinx.com>, git <git@xilinx.com>,
        "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] iio: core: Fix IIO_VAL_FRACTIONAL calculation for
 negative values
Thread-Topic: [PATCH v2] iio: core: Fix IIO_VAL_FRACTIONAL calculation for
 negative values
Thread-Index: AQHWe9Uy2J6wql2dV0mNnaIWIGIkoqlPN8YAgAK0ytA=
Date:   Mon, 31 Aug 2020 08:55:26 +0000
Message-ID: <BY5PR02MB6916B943FBBFD6B1733E5C1EA9510@BY5PR02MB6916.namprd02.prod.outlook.com>
References: <1598465676-28912-1-git-send-email-anand.ashok.dumbre@xilinx.com>
 <20200829161900.713541cd@archlinux>
In-Reply-To: <20200829161900.713541cd@archlinux>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 64250474-ebde-4071-9799-08d84d8b9ad5
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Aug 2020 08:55:26.4933
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: K9eBWr94FM96X06gtC3TEje3DXGtk1K3mzXmXKzSCg8UgcSWFeNKP7T8XeZFtSmS5Wedmib+fwTTQ9EwYDXNTg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR02MB6946
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jonathan,

I encountered this when I was developing a new driver.
If you look at the function where this is used, all other IIO_VAL_MICRO and=
 NANO
have this fix added at some point.

Thanks,
Anand

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, August 29, 2020 4:19 PM
> To: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> Cc: knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; Michal
> Simek <michals@xilinx.com>; git <git@xilinx.com>; linux-
> iio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Anand Ashok Dumbre <ANANDASH@xilinx.com>
> Subject: Re: [PATCH v2] iio: core: Fix IIO_VAL_FRACTIONAL calculation for
> negative values
>=20
> On Wed, 26 Aug 2020 11:14:36 -0700
> Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> wrote:
>=20
> > Fixes IIO_VAL_FRACTIONAL for case when the result is negative and
> > exponent is 0.
> >
> > example: if the result is -0.75, tmp0 will be 0 and tmp1 =3D 75 This
> > causes the output to lose sign because of %d in snprintf which works
> > for tmp0 <=3D -1.
> >
> > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
>=20
> Looks good.  Just one last thing.
>=20
> Is this actually hit in an existing driver?  I'm just wondering how far b=
ack we
> need to push it in stable etc.
>=20
> Thanks,
>=20
> Jonathan
>=20
> > ---
> > changes since v1:
> > 	Changed -%d to -0 to make the fix clearer.
> > 	Removed the email footer.
> > 	Updated the commit description with an example
> > --
> >  drivers/iio/industrialio-core.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-core.c
> > b/drivers/iio/industrialio-core.c index cdcd16f1..a239fa2 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -592,6 +592,7 @@ static ssize_t __iio_format_value(char *buf,
> > size_t len, unsigned int type,  {
> >  	unsigned long long tmp;
> >  	int tmp0, tmp1;
> > +	s64 tmp2;
> >  	bool scale_db =3D false;
> >
> >  	switch (type) {
> > @@ -614,10 +615,13 @@ static ssize_t __iio_format_value(char *buf,
> size_t len, unsigned int type,
> >  		else
> >  			return scnprintf(buf, len, "%d.%09u", vals[0], vals[1]);
> >  	case IIO_VAL_FRACTIONAL:
> > -		tmp =3D div_s64((s64)vals[0] * 1000000000LL, vals[1]);
> > +		tmp2 =3D div_s64((s64)vals[0] * 1000000000LL, vals[1]);
> >  		tmp1 =3D vals[1];
> >  		tmp0 =3D (int)div_s64_rem(tmp, 1000000000, &tmp1);
> > -		return scnprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
> > +		if ((tmp2 < 0) && (tmp0 =3D=3D 0))
> > +			return snprintf(buf, len, "-0.%09u", abs(tmp1));
> > +		else
> > +			return snprintf(buf, len, "%d.%09u", tmp0,
> abs(tmp1));
> >  	case IIO_VAL_FRACTIONAL_LOG2:
> >  		tmp =3D shift_right((s64)vals[0] * 1000000000LL, vals[1]);
> >  		tmp0 =3D (int)div_s64_rem(tmp, 1000000000LL, &tmp1);