[PATCH] staging: mt7621-pci: update driver's TODO file

[NeilBrown]

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 at gmail.com>
> ---
> 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 at brown.name>
> -- 
> 2.19.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20190211/e2c17b35/attachment.asc>