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:   Thu, 13 Dec 2018 15:01:46 -0700
From:   Jeremy Fertic <jeremyfertic@gmail.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     Jonathan Cameron <jic23@kernel.org>, devel@driverdev.osuosl.org,
        Lars-Peter Clausen <lars@metafoo.de>,
        Michael Hennerich <Michael.Hennerich@analog.com>,
        linux-iio@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        Hartmut Knaack <knaack.h@gmx.de>
Subject: Re: [PATCH 04/11] staging: iio: adt7316: fix handling of dac high
 resolution option
Message-ID: <20181213220146.GA2496@r2700x.localdomain>
References: <20181212005503.28054-1-jeremyfertic@gmail.com>
 <20181212005503.28054-5-jeremyfertic@gmail.com>
 <20181212082315.GI3116@kadam>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181212082315.GI3116@kadam>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Dec 12, 2018 at 11:23:16AM +0300, Dan Carpenter wrote:
> On Tue, Dec 11, 2018 at 05:54:56PM -0700, Jeremy Fertic wrote:
> > @@ -651,10 +649,12 @@ static ssize_t adt7316_store_da_high_resolution(struct device *dev,
> >  	u8 config3;
> >  	int ret;
> >  
> > +	if (chip->id == ID_ADT7318 || chip->id == ID_ADT7519)
> > +		return -EPERM;
> 
> return -EINVAL is more appropriate than -EPERM.
> 
> regards,
> dan carpenter
> 

I chose -EPERM because the driver uses it quite a few times in similar
circumstances. At least with this driver, -EINVAL is used when the user
attempts to write data that would never be valid. -EPERM is used when
either the current device settings prevent some functionality from being
used, or the device never supports that functionality. This patch is the
latter, that these two chip ids never support this function.

I'll change to -EINVAL in a v2 series, but I wonder if I should hold off
on a separate patch for other instances in this driver since it will be
undergoing a substantial refactoring. Is there any rule of thumb about
when -EPERM is appropriate for a driver, or is -EINVAL almost always the
better option?