On Sun, Feb 10 2019, Sergio Paracuellos wrote:
> Some of the things included in driver's TODO file have
> been properly achieved. Update file accordly.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> ---
> Hi Neil,
>
> I've been looking through the code of this driver and pci-phy
> driver while thinking in what is missing already to get this
> mainlined out of staging. I don't really know what cleanups
> are missing here. The only thinkg is not clear yet is the thing
> with the clocks defined in device-tree. Should be just remove
> them and give this stuff a try to get feedback or what is missing?
> Can you please point me out in the right direction?
Hi Sergio,
thanks for persisting with this.
I think the "right" thing to do with the clocks is to write a driver
that support ralink,rt2880-clock, similar to the way
arch/mips/ralink/reset.c
supports ralink,rt2880-reset
It would support clk_enable by setting the relevant bit in
#define RALINK_CLKCFG1 0x30
and clk_disable by clearing it.
The pci-mt7621.c (and pci-mt7620.c) could use this to enable/disable
the clocks, rather than poking directly at the register.
Also, pci-mt7621.c (maybe) shouldn't be poking RALINK_PCIE_RST directly.
This is apparently another reset line the driver needs, so it should be
described in devicetree. i.e add another reset-name "pcie".
Also,
#define RALINK_PCI_IO_MAP_BASE 0x1e160000
duplicates info that is in devicetree:
ranges = <
0x02000000 0 0x00000000 0x60000000 0 0x10000000 /* pci memory */
0x01000000 0 0x00000000 0x1e160000 0 0x00010000 /* io space */
^^HERE^^^^
>;
I wonder if that matter...
A tiny thing:
* 3'b100 0 x x
* 3'b101 1 x 0
* 3'b110 1 0 x
* 3'b111 2 1 0
There are two sets of 8 spaces in there, that should be a TAB, and there
is a space before a TAB, that should be removed (yes, I do have x-ray vision).
I really don't know how important all this is. Maybe it would be best
to post the patch and see what people say.
Thanks,
NeilBrown
>
> Thanks in advance for your time.
>
> Best regards,
> Sergio Paracuellos
> drivers/staging/mt7621-pci/TODO | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/staging/mt7621-pci/TODO b/drivers/staging/mt7621-pci/TODO
> index cf30f629b9fd..ccfd266db4ca 100644
> --- a/drivers/staging/mt7621-pci/TODO
> +++ b/drivers/staging/mt7621-pci/TODO
> @@ -1,12 +1,4 @@
>
> - general code review and cleanup
> -- can this be converted to not require PCI_DRIVERS_LEGACY ??
> - The irq returned by pcibios_map_irq is a "hwirq" (hardware irq number)
> - and pci_assign_irq() assigns this directly to dev->irq, which
> - expects a "virq" (virtual irq number). These numbers are different
> - on MIPS. There is a gross hack to make it work on one
> - specific platform, so it can be tested.
> -- Should this be merged with arch/mips/pci/pci-mt7620.c ??
> -- ensure device-tree requirements are documented
>
> Cc: NeilBrown <neil@xxxxxxxxxx>
> --
> 2.19.1
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
