On Thu, 05 Nov 2020, Vaittinen, Matti wrote: > > 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 do see people casting to uintptr and placing the result into a long. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog