Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 12 Jan 2019 17:02:52 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Jeremy Fertic <jeremyfertic@gmail.com>
Cc:     Shreeya Patel <shreeya.patel23498@gmail.com>, lars@metafoo.de,
        Michael.Hennerich@analog.com, knaack.h@gmx.de, pmeerw@pmeerw.net,
        gregkh@linuxfoundation.org, linux-iio@vger.kernel.org,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Staging: iio: adt7316: Add regmap support
Message-ID: <20190112170252.56406302@archlinux>
In-Reply-To: <20190105220413.GA3449@r2700x.localdomain>
References: <20181223140224.10958-1-shreeya.patel23498@gmail.com>
        <20190105172037.7cc5006b@archlinux>
        <20190105220413.GA3449@r2700x.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

...  
> > > diff --git a/drivers/staging/iio/addac/adt7316.h b/drivers/staging/iio/addac/adt7316.h
> > > index fd7c5c92b599..2c72cf3f71cd 100644
> > > --- a/drivers/staging/iio/addac/adt7316.h
> > > +++ b/drivers/staging/iio/addac/adt7316.h
> > > @@ -11,16 +11,13 @@
> > >  
> > >  #include <linux/types.h>
> > >  #include <linux/pm.h>
> > > +#include <linux/regmap.h>
> > >  
> > >  #define ADT7316_REG_MAX_ADDR		0x3F
> > >  
> > > -struct adt7316_bus {
> > > -	void *client;
> > > -	int irq;
> > > -	int (*read)(void *client, u8 reg, u8 *data);
> > > -	int (*write)(void *client, u8 reg, u8 val);
> > > -	int (*multi_read)(void *client, u8 first_reg, u8 count, u8 *data);
> > > -	int (*multi_write)(void *client, u8 first_reg, u8 count, u8 *data);
> > > +static const struct regmap_config adt7316_regmap_config = {
> > > +	.reg_bits = 8,
> > > +	.val_bits = 10,  
> 
> I wonder if val_bits should be 8. The driver can read and write 8, 10,
> or 12 bit values. In the 10 and 12 bit cases, the driver currently
> (including with this patch) does two separate reads or writes and expects
> an 8 bit result from each. It then parses these two values to come up with
> the 10 or 12 bit value. I don't think the logic for this calculation is
> known to regmap, so with the current form of the patch, I think val_bits
> should be 8. Maybe there is a better way to do it though?

Good point. I would assume they should be 8 as well.


> 
> I would have tested this but I couldn't get the patch to apply. Shreeya,
> I think if you rebase against iio/testing that might take care of it. I
> can then do some testing with v2.
>
Great.

Jonathan 
> Jeremy