On Sun, Jul 8, 2018 at 5:36 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Jul 07, 2018 at 09:48:37PM +0200, Sergio Paracuellos wrote:
>> This commit simplifies and clean a lot of stuff related with pci
>> reads and writes. It deletes a lot of not needed at all functions
>> and use kernel arch operations read[b,w,l] and write[b,w,l] instead
>> of use custom macros. It also include one function helper called
>> 'mt7621_pci_get_cfgaddr' to easily obtain config address. Also to
>> get pci base address a global 'mt7621_pci_base' variable has been
>> included and initialized as a pointer to RALINK_PCI_BASE in driver
>> probe function. With this changes LOC is clearly decreased and
>> readability is increased.
>
> A lot of different things are happening here in this patch, making it
> hard to review. Any chance to split this up into smaller, easier to
> review, parts?
It can be but all changes are really related and this just delete all of those
crazy read and write functions and adding some helpers in the way to
make easier the rewrite of real read and write.
>
> And you adding mt7621_pci_base is a nice start, but that really should
> be a device-specific variable, not a global one. I can't belive this
> driver works with a hard-coded base address, that's crazy... Shouldn't
> that value be read from the PCI device itself instead?
I know this shouldn't be a global variable but, as you said, is a nice start to
make this driver a little cleaner for be able to do a better cleanups
series. Also
all of a lot of hardcoded values should be read from device tree in
next cleanups
in order to have a cleaner driver.
Also this patch is the first in my next series and as you can see it
contains very ugly hacks, so
I though to do a first simple cleanups first for finally end up with
easier real cleans. Maybe that
is not the best approach.
I can try to split this a bit but I don't really know how to do atomic
changes because
of the related things included here.
>
> thanks,
>
> greg k-h
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
