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;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Fri, 24 Sep 2021 04:35:47 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Paul Fertser <fercerpav@gmail.com>
Cc:     linux-hwmon@vger.kernel.org, Jean Delvare <jdelvare@suse.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] hwmon: tmp421: handle I2C errors
Message-ID: <20210924113547.GA2694238@roeck-us.net>
References: <20210922155323.GA3205709@roeck-us.net>
 <20210923094801.23332-1-fercerpav@gmail.com>
 <20210924022020.GA3032273@roeck-us.net>
 <20210924074450.GU23326@home.paul.comp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210924074450.GU23326@home.paul.comp>
Precedence: bulk

On Fri, Sep 24, 2021 at 10:44:50AM +0300, Paul Fertser wrote:
> Hello Guenter,
> 
> On Thu, Sep 23, 2021 at 07:20:20PM -0700, Guenter Roeck wrote:
> > > --- a/drivers/hwmon/tmp421.c
> > > +++ b/drivers/hwmon/tmp421.c
> > > @@ -93,7 +93,7 @@ struct tmp421_data {
> > >  	struct hwmon_channel_info temp_info;
> > >  	const struct hwmon_channel_info *info[2];
> > >  	struct hwmon_chip_info chip;
> > > -	char valid;
> > > +	bool valid;
> > 
> > This is a cleanup, not a bug fix, and it isn't even mentioned
> > in the patch description.
> 
> For the bug fix I needed to add a line setting data->valid to false
> and was reluctant to assign 0 instead as it would be adding more wrong
> (type-wise) code. But since you say that's the way to go, sure, will
> do.

I figured that much, but it is still an unrelated change. I don't object
to making that change, but it should still be a separate patch. FWIW,
I don't mind if it is in a single patch if the code won't be applied
to stable releases, but it should still be mentioned in the description
(eg: while at it, convert 'valid' to boolean).

> 
> Regarding the cleanup, I see hwmon has many examples of "char valid"
> and so it might make sense to do them all at once. What would be the
> procedure here, is that a desired change at all and if yes, how to
> approach doing it? 

You could write a coccinelle script to detect and convert all uses,
then submit a single patch to convert all drivers, with the cocconelle
script as reference in the commit log.

Guenter