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@xxxxxxxxxxxxxxxxx > > > > > --- > > > > > + 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
