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:

quoted

On Thu, 2020-11-05 at 08:46 +0200, Matti Vaittinen wrote:

quoted

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:

quoted

On Wed, 28 Oct 2020, Matti Vaittinen wrote:

quoted

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@fi.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