linux-kernel - Re: [PATCH v4 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c969857d70c3fea3e5369651d09aa4ba668418f7.camel@fi.rohmeurope.com>
Date:   Thu, 5 Nov 2020 08:56:17 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "lee.jones@...aro.org" <lee.jones@...aro.org>
CC:     "linux@...ck-us.net" <linux@...ck-us.net>,
        "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        linux-power <linux-power@...rohmeurope.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>
Subject: Re: [PATCH v4 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF


On Thu, 2020-11-05 at 08:21 +0000, Lee Jones wrote:
> On Thu, 05 Nov 2020, Vaittinen, Matti wrote:
> 
> > On Thu, 2020-11-05 at 08:46 +0200, Matti Vaittinen wrote:
> > > Morning Lee,
> > > 
> > > Thanks for taking a look at this :) I see most of the comments
> > > being
> > > valid. There's two I would like to clarify though...
> > > 
> > > On Wed, 2020-11-04 at 15:51 +0000, Lee Jones wrote:
> > > > On Wed, 28 Oct 2020, Matti Vaittinen wrote:
> > > > 
> > > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which
> > > > > are
> > > > > mainly used to power the R-Car series processors.
> > > > > 
> > > > > Signed-off-by: Matti Vaittinen <
> > > > > matti.vaittinen@...rohmeurope.com
> > > > > ---
> > > > > +	unsigned int chip_type;
> > > > > +
> > > > > +	chip_type = (unsigned int)(uintptr_t)
> > > > > +		    of_device_get_match_data(&i2c->dev);
> > > > 
> > > > Not overly keen on this casting.
> > > > 
> > > > Why not just leave it as (uintptr_t)?
> > > 
> > > I didn't do so because on x86_64 the address width is probably 64
> > > bits
> > > whereas the unsigned int is likely to be 32 bits. So the
> > > assignment
> > > will crop half of the value. It does not really matter as values
> > > are
> > > small - but I would be surprized if no compilers/analyzers
> > > emitted a
> > > warning.
> > > 
> > > I must admit I am not 100% sure though. I sure can change this if
> > > you
> > > know it better?
> 
> What if you used 'long', which I believe changed with the
> architecture's bus width in Linux?

I think this is exactly what uintptr_t was created for. To provide type
which assures a pointer conversion to integer and back works.

I guess I can change the

unsigned int chip_type;

to uintptr_t and get away with single cast if it looks better to you.
For me the double cast does not look that bad when it allows use of
native int size variable - but in this case it's really just a matter
of taste. Both should work fine.

I'll cook v5.

--Matti