On Thu, Jan 23, 2025 at 02:09:24PM -0500, Frank Li wrote: > On Thu, Jan 23, 2025 at 07:35:06PM +0100, Niklas Cassel wrote: > > > > > > Actually, you change code logic although functionality is the same. I feel > > > like you should mention at commit message or use origial code by just > > > change variable type. > > > > > > #ifdef CONFIG_PHYS_ADDR_T_64BIT > > > typedef u64 phys_addr_t; > > > #else > > > typedef u32 phys_addr_t; > > > #endif > > > > Hello Frank, > > > > I personally think that is a horrible idea :) > > > > We do not want to introduce ifdefs in the middle of the code, unless > > in exceptional circumstances, like architecture specific optimized code. > > You miss understand what my means. I copy it from type.h to indicate > resource_size_t is not 64bit at all platforms. I know that resource_size_t is typedefed to phys_addr_t, which can be 32-bit or 64-bit. (I compile tested this patch on 32-bit both with and without PAE.) resource_size_t is the type returned by pci_resource_len(). That is why the patch in subject changes the type to use resource_size_t. IMO, it does not make sense to use any other type (e.g. u64), since the value returned by pci_resource_len() will still be limited to what can be represented by resource_size_t. A BARs larger than 4GB, on systems with 32-bit resource_size_t, will get disabled by PCI core: https://github.com/torvalds/linux/blob/v6.13/drivers/pci/probe.c#L265-L270 So all good. As for your question why I don't keep the division, please read the comment section in this patch (where the changelog usually is), or read the thread: https://lore.kernel.org/linux-pci/20250109094556.1724663-1-18255117159@xxxxxxx/T/#t I guess I could have added: " In order to handle 64-bit resource_type_t on 32-bit platforms, we would have needed to use a function like div_u64() or similar. Instead, change the code to use addition instead of division. This avoids the need for div_u64() or similar, while also simplifying the code. " Let me send a V2 with that senctence added to address your review comment. Kind regards, Niklas
